This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Add LocationContext to SymbolMetadata
ClosedPublic

Authored by a.sidorin on Jul 4 2016, 10:22 AM.

Details

Summary

Currently, SymbolMetadata are distinguished by statement and block count. However, in case of inlining, block counts may be the same every time function is called because the counter restarts from zero (it is related to StackFrameContext which is always different). This leads to unexpected resurrection of SymbolMetadata.

This patch solves this problem by adding a LocationContext to SymbolMetadata. A simple test is provided as well.

Diff Detail

Repository
rL LLVM

Event Timeline

a.sidorin updated this revision to Diff 62696.Jul 4 2016, 10:22 AM
a.sidorin retitled this revision from to [analyzer] Add LocationContext to SymbolMetadata.
a.sidorin updated this object.
a.sidorin added reviewers: zaks.anna, dcoughlin.
a.sidorin added a subscriber: cfe-commits.
NoQ added a subscriber: NoQ.Jul 6 2016, 4:17 PM
NoQ added a comment.Jul 6 2016, 5:32 PM

I also never noticed that the block count gets reset on every stack frame, that's pretty unobvious. Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count itself seems to be pretty complicated.

Also, maybe once the ScopeContext thing is ready, we'd be able to omit the block count completely.

In D21978#476200, @NoQ wrote:

I also never noticed that the block count gets reset on every stack frame, that's pretty unobvious.

Yes, that was surprising for me too.

Uhm, and it's already there in SymbolConjured. Anyway, fixing the block count itself seems to be pretty complicated.

Also, maybe once the ScopeContext thing is ready, we'd be able to omit the block count completely.

Not sure. According to review comments, we may want for ScopeContext to be created for scopes with variables declared only (it seems reasonable). So ScopeContext may cover multiple blocks.

NoQ accepted this revision.Aug 17 2016, 8:08 AM
NoQ added a reviewer: NoQ.
This revision is now accepted and ready to land.Aug 17 2016, 8:08 AM
This revision was automatically updated to reflect the committed changes.