This is an archive of the discontinued LLVM Phabricator instance.

[AA] Handle callbr instructions in alias analysis.
ClosedPublic

Authored by rickyz on Dec 18 2021, 8:05 AM.

Details

Summary

Before this change, AAResults::getModRefInfo() was missing a case for
callbr instructions (asm goto), which may read/write memory. In PR52735,
this led to a miscompile where a load was incorrect eliminated.

Add this missing case, as well as an assert verifying that all
memory-accessing instructions are handled properly.

Fixes PR52735.

Diff Detail

Unit TestsFailed

Event Timeline

rickyz created this revision.Dec 18 2021, 8:05 AM
rickyz requested review of this revision.Dec 18 2021, 8:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 18 2021, 8:05 AM
nikic accepted this revision.Dec 18 2021, 8:58 AM

LGTM

llvm/lib/Analysis/AliasAnalysis.cpp
698–701

As an optional suggestion, I think this would make it clearer which overload gets actually called.

llvm/test/Analysis/BasicAA/pr52735.ll
15

Just CHECK suffices here.

This revision is now accepted and ready to land.Dec 18 2021, 8:58 AM
rickyz updated this revision to Diff 395296.Dec 18 2021, 9:02 AM
rickyz marked 2 inline comments as done.

Respond to comments.

Thanks for the review! I'm not a committer, so can you please commit this if it looks good?

nikic added a comment.Dec 18 2021, 9:04 AM

Thanks for the review! I'm not a committer, so can you please commit this if it looks good?

Sure, can you please share Your Name <your@email> to use for attribution?

Sure thing:

Ricky Zhou <ricky@rzhou.org>

This revision was landed with ongoing or failed builds.Dec 18 2021, 9:52 AM
This revision was automatically updated to reflect the committed changes.