[PATCH] D117244: [Bitcode] [ThinLTO] Add a new bitcode module record for THINLTO_INDEX_FLAG

[Mingming Liu via Phabricator via llvm-commits]

luna added inline comments.


================
Comment at: llvm/tools/llvm-dis/llvm-dis.cpp:194-204
       BitcodeLTOInfo LTOInfo = ExitOnErr(MB.getLTOInfo());
+
+      std::unique_ptr<Module> M;
+      if ((!DumpThinLTOIndexOnly) && (!LTOInfo.OnlyHasThinLTOIndex)) {
+        M = ExitOnErr(
+            MB.getLazyModule(Context, MaterializeMetadata, SetImporting));
+        if (MaterializeMetadata)
----------------
(Just a message before fixing the failed test)

In pre-merge test, `llvm-dis` parsing behavior changed for a bitcode that's invalid itself; the behavior changed from parse error to crash (test expects `llvm-dis` to return parse error while now `llvm-dis` crashes).

After a quick test, this is caused by the change of read order here;
- before this patch, `getLazyModule` will exit-on-err so `getLTOInfo` won't be executed
- after this patch, `getLTOInfo` is executed first, and crashes upon invalid abbrev ID around https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Bitstream/BitstreamReader.h#L528

I'll see how to solve this without changing test expectation from `not` to `crash` (i.e., `llvm-dis` still reports error rather than crash upon invalid bitcode).

The relevant checker line in `invalid.test` is `/var/lib/buildkite-agent/builds/llvm-project/build/bin/llvm-dis -disable-output /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Bitcode/Inputs/invalid-pointer-element-type.bc `


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117244/new/

https://reviews.llvm.org/D117244