[PATCH 03/11] Record a pointer to the AliasSet

[Tobias Grosser]

On 10/06/2014 13:41, Andreas Simbuerger wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> On 06/10/2014 09:21 AM, Tobias Grosser wrote:
>> On 09/06/2014 02:42, Andreas Simbuerger wrote:
>>> Instead of taking the pointer of the AliasSet reference we
>>> directly retreive a pointer from the AliasSetTracker. This gives
>>> us a pointer that should be valid at least until the
>>> ScopDetection is destroyed. --- lib/Analysis/ScopDetection.cpp |
>>> 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/Analysis/ScopDetection.cpp
>>> b/lib/Analysis/ScopDetection.cpp index cab7af7..fbaf07c 100644
>>> --- a/lib/Analysis/ScopDetection.cpp +++
>>> b/lib/Analysis/ScopDetection.cpp @@ -471,8 +471,15 @@ bool
>>> ScopDetection::isValidMemoryAccess(Instruction &Inst, // alias,
>>> if -basicaa is not available. They actually do not, but as we
>>> can // not proof this without -basicaa we would fail. We disable
>>> this check to // not cause irrelevant verification failures. -
>>> if (!AS.isMustAlias()) -    return invalid<ReportAlias>(Context,
>>> /*Assert=*/true, &Inst, &AS); +  if (!AS.isMustAlias()) { +    //
>>> If we want to be able to use this for diagnostics outside of +
>>> // ScopDetection, we need to fetch the AliasSet as a pointer as
>>> soon as it +    // is created above. +    AliasSet *ASPtr =
>>> Context.AST.getAliasSetForPointerIfExists( +        BaseValue,
>>> AliasAnalysis::UnknownSize, +
>>> Inst.getMetadata(LLVMContext::MD_tbaa)); +    return
>>> invalid<ReportAlias>(Context, /*Assert=*/false, &Inst, ASPtr); +
>>> }
>>
>> I am slightly confused. Are you saying this will yield another
>> pointer than deriving the pointer via '&AS'? Why is this the case?
>> Looking at the implementations of getAliasSetForPointer() and
>> getAliasSetForPointerIfExists() both call findAliasSetForPointer(),
>> the first returning the plain pointer, the second dereferencing it
>> and returning it as reference.
>
>
> Ah you're right. For some odd reason it fixed a problem in my jit
> compiler. However, this is not the case anymore, weird.
>
> The problem here is that the reference is only valid as long as the
> DetectionContext is valid. So this means a segfault for any user of
> the RejectLog, when he tries to get the debug message.
>
> I wonder how it fixed anything in the first place... I'll come up with
> something better. Maybe just unpacking the Values from the AliasSet on
> diagnostic object construction.

I just run into this bug again. My understanding is that the AST is 
invalidated with each DetectionContext, I wonder if we should just have 
a single in the ScopDetection that is initialized at the beginning of 
runOnFunction. To my understanding this should fix the issue.

Cheers,
Tobias