[cfe-dev] Weak function pointers (was "SymbolRef and SVal confusion")

[Richard]

I have no great ideas about the new symbol names I'm afraid. I did notice something odd about the current implementation while writing a test though, it seems that the symbol extent range is purged after entering an IfStmt. For example, the following test code will show that the state of weakFunc is unknown in both cases:

void weakFunc() __attribute__((weak_import));
void testWeakFunc() {
    clang_analyzer_eval(weakFunc == NULL);   // expected UNKNOWN
    if (weakFunc == NULL) {
        clang_analyzer_eval(weakFunc == NULL);   // expected TRUE, actually UNKNOWN
    }
}

Should some code be added to SymbolReaper::isLiveRegion to handle this? At the moment this is always returning false for SymbolExtents I think.

On 21 Jan 2013, at 18:48, Jordan Rose <jordan_rose at apple.com> wrote:

> 
> On Jan 20, 2013, at 9:00 , Richard <tarka.t.otter at googlemail.com> wrote:
> 
>>>> +
>>>> +  void setWeakSymbol(const SymbolExtent *S) {
>>>> +    // Only weak functions can have symbols.
>>>> +    assert(cast<ValueDecl>(FD)->isWeak());
>>>> +    WeakSym = S;
>>>> +  }
>>> 
>>> MemRegions need to be immutable, so the setWeakSymbol() is not going to work.  Can we just have the symbol associated with the FunctionTextRegion when it gets created?
>> 
>> I would like to do this, but I don't see how it is possible easily. The SymbolExtent requires the FunctionTextRegion in its constructor, and I don't see a nice way to get hold of the SymbolManager in the FunctionTextRegions constructor to create it. Any suggestions here?
> 
> My new thought on this is that this should just be created lazily by SValBuilder and that it doesn't even need to live on the region. SValBuilder::getWeakSymbolForRegion, or something like that. 
> 
> I'll talk to Anna and Ted about what to rename SymbolExtent and SymbolMetadata to properly include this new usage; if you have any opinions, I'd love to hear them!
> 
> SymbolExtent:
> - based on the region itself
> - does not get invalidated
> - live as long as the region is live (in theory)
> 
> SymbolMetadata:
> - based on the region contents
> - is invalidated along with region contents
> - live as long as the region is live and some checker is interested
> 
> Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130121/a05f8c1a/attachment.html>