[PATCH] D38833: BitstreamWriter: Better assertion when a block's abbrev code size is too small

[Jordan Rose via Phabricator via llvm-commits]

jordan_rose created this revision.

Before: `((Val & ~(~0U >> (32-NumBits))) == 0 && "High bits set!")`
After: `(Abbrev < (1U << CurCodeSize) && "Block code length is too small")`

Should be non-controversial, but maybe there's some scenario where the old assertion wouldn't have actually tripped? (I was originally going to put this in EmitAbbrev instead, but realized that it's possible for some overly clever soul to have a block that just never uses high-abbrev records.)


Repository:
  rL LLVM

https://reviews.llvm.org/D38833

Files:
  include/llvm/Bitcode/BitstreamWriter.h
  unittests/Bitcode/BitstreamWriterTest.cpp


Index: unittests/Bitcode/BitstreamWriterTest.cpp
===================================================================
--- unittests/Bitcode/BitstreamWriterTest.cpp
+++ unittests/Bitcode/BitstreamWriterTest.cpp
@@ -56,4 +56,33 @@
   EXPECT_EQ(StringRef("str0"), Buffer);
 }
 
+#ifndef NDEBUG
+TEST(BitstreamWriterTest, trapOnSmallAbbrev) {
+  SmallString<64> Buffer;
+  BitstreamWriter W(Buffer);
+  W.EnterSubblock(bitc::FIRST_APPLICATION_BLOCKID, /*CodeLen*/2);
+  auto Abbrev = std::make_shared<BitCodeAbbrev>();
+  Abbrev->Add(BitCodeAbbrevOp(42U));
+  unsigned AbbrevCode = W.EmitAbbrev(std::move(Abbrev));
+  EXPECT_DEATH({ W.EmitRecordWithAbbrev(AbbrevCode, makeArrayRef(42U)); },
+               "Block code length is too small");
+  W.ExitBlock();
+}
+
+TEST(BitstreamWriterTest, trapOnSmallAbbrevUsingBlockInfo) {
+  SmallString<64> Buffer;
+  BitstreamWriter W(Buffer);
+  W.EnterBlockInfoBlock();
+  auto Abbrev = std::make_shared<BitCodeAbbrev>();
+  Abbrev->Add(BitCodeAbbrevOp(42U));
+  unsigned AbbrevCode = W.EmitBlockInfoAbbrev(bitc::FIRST_APPLICATION_BLOCKID,
+                                              std::move(Abbrev));
+  W.ExitBlock();
+  W.EnterSubblock(bitc::FIRST_APPLICATION_BLOCKID, /*CodeLen*/2);
+  EXPECT_DEATH({ W.EmitRecordWithAbbrev(AbbrevCode, makeArrayRef(42U)); },
+               "Block code length is too small");
+  W.ExitBlock();
+}
+#endif
+
 } // end namespace
Index: include/llvm/Bitcode/BitstreamWriter.h
===================================================================
--- include/llvm/Bitcode/BitstreamWriter.h
+++ include/llvm/Bitcode/BitstreamWriter.h
@@ -300,6 +300,7 @@
     unsigned BlobLen = (unsigned) Blob.size();
     unsigned AbbrevNo = Abbrev-bitc::FIRST_APPLICATION_ABBREV;
     assert(AbbrevNo < CurAbbrevs.size() && "Invalid abbrev #!");
+    assert(Abbrev < (1U << CurCodeSize) && "Block code length is too small");
     const BitCodeAbbrev *Abbv = CurAbbrevs[AbbrevNo].get();
 
     EmitCode(Abbrev);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38833.118731.patch
Type: text/x-patch
Size: 1970 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171012/7c604cdf/attachment.bin>