This is an archive of the discontinued LLVM Phabricator instance.

Verifier: Simplify and fix issue where we were not verifying unmaterialized functions.
ClosedPublic

Authored by pcc on Jun 6 2016, 2:21 PM.

Details

Summary
  • Arrange to call verify(Function &) on each function, followed by verify(Module &), whether the verifier is being used from the pass or from verifyModule(). As a side effect, this fixes an issue that caused us not to call verify(Function &) on unmaterialized functions from verifyModule().
  • Remove previously unreachable code that verifies that a function definition has an entry block. By definition, a function definition has at least one block.

Diff Detail

Event Timeline

pcc updated this revision to Diff 59786.Jun 6 2016, 2:21 PM
pcc retitled this revision from to Verifier: Simplify and fix issue where we were not verifying unmaterialized functions..
pcc updated this object.
pcc added a reviewer: aprantl.
pcc added a subscriber: llvm-commits.
pcc updated this revision to Diff 59788.Jun 6 2016, 2:29 PM
  • Now that we can verify external functions, remove the assert from verifyFunction()
aprantl added a subscriber: dexonsmith.
aprantl edited edge metadata.Jun 6 2016, 3:02 PM

Would it be possible to add a test for the unmaterialized functions? Sounds like it could be hard though.
The removal of the useless check for the entry block is LGTM as a separate NFC commit.

pcc updated this revision to Diff 59796.Jun 6 2016, 3:50 PM
pcc edited edge metadata.

Add test

pcc added a comment.Jun 6 2016, 3:51 PM

Would it be possible to add a test for the unmaterialized functions? Sounds like it could be hard though.

Yes, I added one based on function attached metadata.

The removal of the useless check for the entry block is LGTM as a separate NFC commit.

r271948

aprantl added inline comments.Jun 6 2016, 3:59 PM
lib/IR/Verifier.cpp
4482

Can you remind me why this change is only necessary in the LegacyPass implementation?

pcc added inline comments.Jun 6 2016, 4:04 PM
lib/IR/Verifier.cpp
4482

The LegacyPass implementation is the only one that calls verify(Function &) and verify(Module &) directly. The new pass manager uses the verifyModule and verifyFunction functions.

aprantl accepted this revision.Jun 6 2016, 4:16 PM
aprantl edited edge metadata.

Thanks, I think this change is fine, then.

This revision is now accepted and ready to land.Jun 6 2016, 4:16 PM
This revision was automatically updated to reflect the committed changes.