Check for invalid abbreviation operators in munged bitcode.

unittests/Bitcode/NaClMungeWriteErrorTests.cpp

Index: unittests/Bitcode/NaClMungeWriteErrorTests.cpp
diff --git a/unittests/Bitcode/NaClMungeWriteErrorTests.cpp b/unittests/Bitcode/NaClMungeWriteErrorTests.cpp
index fa2bec420b3ab7b8370658ca5cf446c3afe08eac..c974385de0a79e52a606cd6e199d4749e5d56ab7 100644
--- a/unittests/Bitcode/NaClMungeWriteErrorTests.cpp
+++ b/unittests/Bitcode/NaClMungeWriteErrorTests.cpp
@@ -38,7 +38,7 @@ const uint64_t BitcodeRecords[] = {
 };
 
 // Expected output when bitcode records are dumped.
-const char* ExpectedDump =
+const char *ExpectedDump =
     "       0:0|<65532, 80, 69, 88, 69, 1, 0,|Magic Number: 'PEXE' (80, 69, "
     "88, 69)\n"
     "          | 8, 0, 17, 0, 4, 0, 2, 0, 0, |PNaCl Version: 2\n"
@@ -60,6 +60,24 @@ const char* ExpectedDump =
     "      64:0|0: <65534>                   |}\n"
     ;
 
+// Same as above, but just the records as generated by NaClWriteMunger.

[jvoung (off chromium), 2015/05/27 20:09:37]

I don't see a class named NaClWriteMunger.

[Karl, 2015/05/28 20:50:31]

I was trying to clarify what tool generated the output. The dump above is from
class NaClObjDumpMunger while the following is from NaClWriteMunger.

However, after thinking about it a bit more, I realized I could have just used
NaClObjDumpMunger as well. Fixing the code to do that.

Also realized that the check pattern use for abbreviations could be applied
elsewhere, removing code. So I'm also changing that.

+const char *ExpectedRecords =
+    "       1: [65535, 8, 2]\n"
+    "         1: [65535, 17, 3]\n"
+    "           3: [1, 2]\n"
+    "           3: [2]\n"
+    "           3: [21, 0, 0]\n"
+    "         0: [65534]\n"
+    "         3: [8, 1, 0, 0, 0]\n"
+    "         1: [65535, 12, 2]\n"
+    "           3: [1, 1]\n"
+    "           3: [10]\n"
+    "         0: [65534]\n"
+    "       0: [65534]\n";
+
+const char *UnableToContinue =
+      "Error: Unable to generate bitcode file due to write errors\n";
+
 // Edit to change void type with an illegal abbreviation index.
 const uint64_t VoidTypeIndex = 3; // Index for "@t0 = void".
 const uint64_t AbbrevIndex4VoidTypeEdit[] = {
@@ -345,7 +363,7 @@ TEST(NaClMungeWriteErrorsTests, TestWritingMalformedAbbreviation) {
   NaClWriteMunger Munger(ARRAY_TERM(BitcodeRecords));
   EXPECT_FALSE(Munger.runTest(ARRAY(Edits)));
   EXPECT_EQ(
-      "Error (Block 12): Error: Bad abbreviation operand encoding 10:"
+      "Error (Block 12): Bad abbreviation operand encoding 10:"
       " 2: [65533, 1, 10]\n"
       "Error: Unable to generate bitcode file due to write errors\n",
       Munger.getTestResults());
@@ -354,7 +372,7 @@ TEST(NaClMungeWriteErrorsTests, TestWritingMalformedAbbreviation) {
   Munger.setTryToRecoverOnWrite(true);
   EXPECT_TRUE(Munger.runTest(ARRAY(Edits)));
   EXPECT_EQ(
-      "Error (Block 12): Error: Bad abbreviation operand encoding 10: "
+      "Error (Block 12): Bad abbreviation operand encoding 10: "
       "2: [65533, 1, 10]\n"
       "       1: [65535, 8, 2]\n"
       "         1: [65535, 17, 3]\n"
@@ -395,7 +413,7 @@ TEST(NaClMungedWriteErrorTests, TestRemovingAbbrevWithMultAbbrevs) {
   Munger.setTryToRecoverOnWrite(true);
   EXPECT_TRUE(Munger.runTest(ARRAY(Edits)));
   EXPECT_EQ(
-      "Error (Block 12): Error: Bad abbreviation operand encoding 9:"
+      "Error (Block 12): Bad abbreviation operand encoding 9:"
       " 2: [65533, 1, 9]\n"
       "Error (Block 12): Ignoring abbreviation: 2: [65533, 1, 1, 10]\n"
       "Error (Block 12): Uses illegal abbreviation index: 5: [10]\n"
@@ -561,4 +579,130 @@ TEST(NaClMungeWriteErrorTests, RecoverWriteRecordOutsideBlock) {
       Munger.getTestResults());
 }
 
+// Runs write munging tests on BitcodeRecords, inserting bad
+// abbreviation definitions via Edit.  ErrorMessage is the expected

[jvoung (off chromium), 2015/05/27 20:09:37]

via Edit -> via Edits ?

[Karl, 2015/05/28 20:50:32]

Done.

+// error message to be generated by the write munger. Assumes the the

[jvoung (off chromium), 2015/05/27 20:09:37]

the the -> that the

[Karl, 2015/05/28 20:50:31]

Done.

+// error recovery is to delete the bad abbreviation definitions.

[jvoung (off chromium), 2015/05/27 20:09:37]

error recover -> error recover method

or something like that?

[Karl, 2015/05/28 20:50:31]

No longer applicable. Comments were rewritten.

+void CheckWriteBadAbbrevMunging(const uint64_t *Edits, size_t EditsSize,
+                                const char *ErrorMessage) {
+  NaClWriteMunger Munger(ARRAY_TERM(BitcodeRecords));
+  EXPECT_FALSE(Munger.runTest(Edits, EditsSize));
+  std::string BadResults(ErrorMessage);
+  BadResults.append(UnableToContinue);
+  EXPECT_EQ(BadResults, Munger.getTestResults());
+
+  Munger.setTryToRecoverOnWrite(true);
+  EXPECT_TRUE(Munger.runTest(Edits, EditsSize));
+  std::string GoodResults(ErrorMessage);
+  GoodResults.append(ExpectedRecords);
+  EXPECT_EQ(GoodResults, Munger.getTestResults());
+}
+
+// Show that inserting an abbreviation with a bad fixed width is dealt with.
+TEST(NaClMungeWriteErrorTests, InvalidFixedAbbreviationSize) {
+  // Insert bad abbreviation Fixed(36) into type block.

[jvoung (off chromium), 2015/05/27 20:09:37]

how about static_assert(36 > naclbitc::MaxAbbrevWidth, "...");

To show why 36 is chosen.

[Karl, 2015/05/28 20:50:31]

Done.

+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 1,
+      0, NaClBitCodeAbbrevOp::Fixed, 36, Terminator

[jvoung (off chromium), 2015/05/27 20:09:38]

can line up the 0, ... with the other array elems from the earlier rows

[Karl, 2015/05/28 20:50:32]

Done.

+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Invalid abbreviation Fixed(36) in: 2: [65533, 1, 0,"
+      " 1, 36]\n");
+}
+
+// Show that inserting an abbreviation with a bad vbr width is dealt with.
+TEST(NaClMungeWriteErrorTests, InvalidVbrAbbreviationSize) {
+  // Insert bad abbreviation Vbr(36) into type block.
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 1,
+    0, NaClBitCodeAbbrevOp::VBR, 36,
+    Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Invalid abbreviation VBR(36) in: 2: [65533, 1, 0,"
+      " 2, 36]\n");
+}
+
+// Show that the array operator can't appear last.
+TEST(NaClMungeWriteErrorTests, InvalidArrayAbbreviationLast) {
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 1,
+    0, NaClBitCodeAbbrevOp::Array, Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Malformed array abbreviation found: 2: [65533, 1,"
+      " 0, 3]\n");
+}
+
+// Show that the array operator can't appear before the second to last
+// operand.
+TEST(NaClMungeWriteErrorTests, InvalidArrayAbbreviationTooEarly) {
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 3,
+    0, NaClBitCodeAbbrevOp::Array,  // array
+    1, 15,                          // lit(15)
+    1, 10,                          // lit(10)
+    Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Array abbreviation must be second to last: 2: [65533,"
+      " 3, 0, 3, 1, 15, 1, 10]\n");
+}
+
+// Show that the array operator can't appear as last two operators.
+TEST(NaClMungeWriteErrorTests, InvalidArrayAbbreviationLastTwo) {
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 2,
+    0, NaClBitCodeAbbrevOp::Array,  // array
+    0, NaClBitCodeAbbrevOp::Array,  // array
+    Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Malformed array abbreviation found: 2: [65533, 2,"
+      " 0, 3, 0, 3]\n");
+}
+
+// Show that the code checks if specifies too few operands for an

[jvoung (off chromium), 2015/05/27 20:09:37]

nit: "Show that the code checks if specifies too few operands" sounds a bit odd.

[Karl, 2015/05/28 20:50:31]

Rewritten.

+// abbreviation, based on the record size.
+TEST(NaClMungeWriteErrorTests, SpecifiesTooFewOperands) {
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 1,
+    1, 10,                          // lit(10)
+    1, 15,                          // lit(15)
+    Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Error: Too many values for number of operands (1):"
+      " 2: [65533, 1, 1, 10, 1, 15]\n");
+}
+
+// Show that the code checks if specifies too many operands for an
+// abbreviation, based on the record size.
+TEST(NaClMungeWriteErrorTests, SpecifiesTooManyOperands) {
+  // Insert bad abbreviation Vbr(36) into type block.
+  const uint64_t Edit[] = {
+    VoidTypeIndex, NaClMungedBitcode::AddBefore,
+    naclbitc::DEFINE_ABBREV, naclbitc::BLK_CODE_DEFINE_ABBREV, 3,
+    1, 10,                          // lit(10)
+    1, 15,                          // lit(15)
+    Terminator
+  };
+  CheckWriteBadAbbrevMunging(
+      ARRAY(Edit),
+      "Error (Block 17): Malformed abbreviation found. Expects 3 operands"
+      " but found 2: 2: [65533, 3, 1, 10, 1, 15]\n");
+}
+
 } // end of namespace naclmungetest