[PATCH] D38201: Use a BumpPtrAllocator for Loop objects

Chandler Carruth via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 23:29:01 PDT 2017


chandlerc added a comment.

In https://reviews.llvm.org/D38201#881716, @sanjoy wrote:

> In https://reviews.llvm.org/D38201#880864, @chandlerc wrote:
>
> > In https://reviews.llvm.org/D38201#880392, @sanjoy wrote:
> >
> > > There is a problem with this patch -- the legacy pass manager checks if a loop is valid or not by querying `isInvalid`, which is incorrect; since we call the destructor for loop objects to mark them as "invalid" calling `isInvalid` for possibly invalid loop objects has UB.  We already have this UB in `User::operator delete` though, and so given that and that the legacy pass manager is going away at some point, perhaps this is okay?  Alternatively, I could add a mechanism on the legacy loop pass manager that lets passes denote loop objects to have been deleted (by remembering the pointers in a set) and use that to decide if a Loop object is valid or not (and with that, I can guard `isInvalid` with `#ifndef NDEBUG`).
> >
> >
> > Sadly, I think this would be problematic. I think it'll trip ASan's use-after-scope or use-after-lifetime checks.... =/ But maybe there is a way around that. Or maybe just implement the solution you suggest. Dunno, but definitely worth checking.
>
>
> We already do this for `User` instances -- we read from fields in the `User` object to be destroyed in its `operator delete`, after the destructors have run.  However,  two wrongs don't make a right so I'll first try to see if I can fix this in the old pass manager.


Cool...

However, to be clear, I'm not trying to argue any theorectical purity here. Mostly, I'm worried that the operator delete may be somewhat less problematic *in practice*. If you have carefully verified that this causes no problems in practice with lifetime-markers and sanitizers as they are currently implemented.


https://reviews.llvm.org/D38201





More information about the llvm-commits mailing list