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

[Jordan Rose via Phabricator via llvm-commits]

jordan_rose requested review of this revision.
jordan_rose added a comment.

Thanks, Roman. Too bad this is such a stable piece of LLVM, or else I'd have more people qualified to review!



================
Comment at: include/llvm/Bitcode/BitstreamWriter.h:303
     assert(AbbrevNo < CurAbbrevs.size() && "Invalid abbrev #!");
+    assert(Abbrev < (1U << CurCodeSize) && "Block code length is too small");
     const BitCodeAbbrev *Abbv = CurAbbrevs[AbbrevNo].get();
----------------
lebedev.ri wrote:
> jordan_rose wrote:
> > lebedev.ri wrote:
> > > What is the expected range of `CurCodeSize` values?
> > > This has UB if `CurCodeSize` == `32`.
> > > It would be much safer to just use the variant with subtraction:
> > > ```
> > > assert(Abbrev <= (~(~0U >> (32-CurCodeSize))) && "Block code length is too small");
> > > ```
> > Good point. I don't think anyone's trying to use 32-bit record codes, but the [spec](http://llvm.org/docs/BitCodeFormat.html) doesn't seem to impose a limit. I'll switch it.
> I think you want to move this assert to before the first use of `Abbrev` (before line 301)
It shouldn't matter in this case. The first use (and the above assertion) is for indexing into a write-time data structure; the second is for the actual representation in the bitstream.


Repository:
  rL LLVM

https://reviews.llvm.org/D38833