[PATCH] D102803: [Verifier] Remove some redundant checks

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:5146-5149
+      if (FailureOrdering == AtomicOrdering::NotAtomic ||
+          FailureOrdering == AtomicOrdering::Unordered)
+        return error("Invalid record");
+
----------------
aeubanks wrote:
> dblaikie wrote:
> > Does this have test coverage?
> no, it's the thing mentioned in the description
> 
> adding test coverage would require a bitcode file that was generated by a modified LLVM that allows this to bypass the LLParser/verifier
> 
> so this probably isn't an issue in practice since it's always been invalid, so there shouldn't be any bitcode out there that hits this
> I guess I could just remove this check
We should still handle invalid bitcode/shouldn't be able to create undefined situations through invalid bitcode.

So I'd say this needs a test - I assume there are other invalid bitcode tests somewhere around ?

Happy to help create one if you like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102803