[Bitcode] Mention that a multi-module bitcode is valid, each module has exactly one MODULE_BLOCK block

[mingmingl]

Update bitcode format to mention multi-module bitcode.

[tejohnson]

lgtm on the documentation change but that should be submitted separately as it is unrelated to the code fix.
The other change lgtm but if it is feasible to add a test that would be ideal.

llvm/docs/BitCodeFormat.rst

560

Can you fix the line length? The file seems to stick with 80 col.

llvm/lib/Bitstream/Reader/BitstreamReader.cpp

100

Is it feasible to add a test? What was the behavior without the fix?

[mingmingl]

Keep the documentation change and put bitcode reader change to https://reviews.llvm.org/D117630 (which is still a draft).

[mingmingl]

In D117067#3245194, @tejohnson wrote:

lgtm on the documentation change but that should be submitted separately as it is unrelated to the code fix.
The other change lgtm but if it is feasible to add a test that would be ideal.

thanks! I revise this patch to be doc-only, and reformat, and going to pursue bitcode reader change in https://reviews.llvm.org/D117630

The patch is still a draft and I'll see how to add a test (probably an invalid bitcode is needed to trigger the condition);

What's the behavior without the fix

I think upon hitting the branch, trying to call get will run into assertions (with the asserted condition being false); yet it happens very rarely (if at all) since most of time, bitcode file should be valid.

I'll see if there is a lightweight way to construct the bitcode to have a test (maybe by hacking bitcode writer to generate an invalid record).

[tejohnson]

lgtm

llvm/docs/BitCodeFormat.rst

 * 23 --- `STRTAB_BLOCK`_ --- The bitcode file's string table.
 
 .. _MODULE_BLOCK:
 
 MODULE_BLOCK Contents
 ---------------------
 
 The ``MODULE_BLOCK`` block (id 8) is the top-level block for LLVM bitcode files,
-and each bitcode file must contain exactly one. In addition to records
+and each module in a bitcode file must contain exactly one. A bitcode file with multi-module bitcode is valid. In addition to records
 (described below) containing information about the module, a ``MODULE_BLOCK``
 block may contain the following sub-blocks:
 
 * `BLOCKINFO`_
 * `PARAMATTR_BLOCK`_
 * `PARAMATTR_GROUP_BLOCK`_
 * `TYPE_BLOCK`_
 * `VALUE_SYMTAB_BLOCK`_

llvm/lib/Bitstream/Reader/BitstreamReader.cpp

Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
   // Skip unabbreviated records by reading past their entries.
   if (AbbrevID == bitc::UNABBREV_RECORD) {
     Expected<uint32_t> MaybeCode = ReadVBR(6);
     if (!MaybeCode)
       return MaybeCode.takeError();
     unsigned Code = MaybeCode.get();
     Expected<uint32_t> MaybeVBR = ReadVBR(6);
     if (!MaybeVBR)
-      return MaybeVBR.get();
+      return MaybeVBR.takeError();
     unsigned NumElts = MaybeVBR.get();
     for (unsigned i = 0; i != NumElts; ++i)
       if (Expected<uint64_t> Res = ReadVBR64(6))
         ; // Skip!
       else
         return Res.takeError();
     return Code;
   }