[PATCH] D127202: [InlineFunction] Call to getUnderlyingObjects inside AddAliasScopeMetadata shall set MaxLookup = 0

[Nikita Popov via Phabricator via llvm-commits]

nikic added reviewers: jeroen.dobbelaere, efriedma.
nikic added a comment.

Okay, I think the core problem here is that the code assumes that the underlying object may be either a) an argument, b) an identified function local object or c) an escape source -- in the sense of only being able to alias a noalias argument if it has been captured earlier. This assumption //does// mostly hold up in practice, because the implementations of getUnderlyingObjects() and CaptureTracking are carefully synchronized (see in particular https://github.com/llvm/llvm-project/blob/20ca739701d7b2e2aa4028f1a7853f6d0fa0c412/llvm/lib/Analysis/ValueTracking.cpp#L4446-L4458). Looking through history, I think it might be synchronized for this specific usage actually. Of course, the assumption doesn't hold up if we stop walking underlying objects early, in which case we may alias without the argument being captured.

So this can be fixed in two ways: Avoid the assumption, or avoid stopping the walk early. For the latter case, something to be careful about is that using `MaxLookup=0` also removes infinite loop protection for unreachable code. You'll want to add a test case for that. Likely we'd actually want something like D86669 <https://reviews.llvm.org/D86669> as the implementation, which does avoid infinite loops.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127202