Page MenuHomePhabricator

[analyzer] ExprEngine: Fill the Store with initial value of parameters
Needs ReviewPublic

Authored by Charusso on Jun 3 2020, 1:16 AM.

Details

Reviewers
NoQ
Summary

This patch modifies the ExprEngine::getInitialState() by forcing the
creation of the initial values of the parameters of the first function call.

With that we could point out the origin of such values in bug reporting.

Diff Detail

Event Timeline

Charusso created this revision.Jun 3 2020, 1:16 AM
NoQ added a comment.Jun 3 2020, 3:50 AM

This change in the state adds no new information to the state while also burdening the Store with more data to carry and making debugging harder (not only dumps will now have a bunch of explicit bindings, but also these bindings will be much bulkier being derived symbols).

If you want better tracking, just fix the tracking. You already have all the necessary information in the Store to do that.

clang/test/Analysis/osobject-retain-release.cpp
73

The original message is pretty bad but i don't think it became better. The object wasn't "stored into b" by the user; the user simply called a function. The way you made the Store look as if the object was actively stored didn't help; it's up to the visitor to explain why the store looked the way it looked, it's not the job of the Store to look like something else to appease the visitor.

In D81062#2070823, @NoQ wrote:

This change in the state adds no new information to the state while also burdening the Store with more data to carry and making debugging harder (not only dumps will now have a bunch of explicit bindings, but also these bindings will be much bulkier being derived symbols).

I do not see any debug burden because there were no information to begin the debug. The *new* information occurs at the appropriate place now.

If you want better tracking, just fix the tracking. You already have all the necessary information in the Store to do that.

To fix the tracking we need to provide a NoteTag at the point when we encounter a MemRegion. We need to store these MemRegions so that later we could mark them interesting. If we mark them interesting later we also show such notes. That is why this patch and patches like that are essential to begin this change.

PS: To fix the tracking we also need to fill the members of a record type and remove the LazyCompoundVal. If you think we should not store too much data in the Store then transferring the trackExpressionValue() API to NoteTags is insoluble. We cannot create NoteTags on something which is not in the Store already.