This is an archive of the discontinued LLVM Phabricator instance.

[clang][dataflow] Separate context by frame
AbandonedPublic

Authored by samestep on Jul 26 2022, 11:42 AM.

Details

Summary

This patch enables context-sensitive analysis of multiple different calls to the same function (see the ContextSensitiveSetBothTrueAndFalse example in the TransferTest suite) by moving the DeclToLoc, ExprToLoc, and ThisPointeeLoc fields (along with their associated methods) from the DataflowAnalysisContext class into a new StorageFrame class, replacing them with a Frames map keyed by std::vector<const CallExpr *>. The only other change necessary was to replace the copy-assignment with a call to the new popCall method on Environment, which std::moves some fields but specifically does not move DeclToLoc and ExprToLoc from the callee back to the caller.

This approach still does not yet work for recursive calls, such as the example pointed out by @xazax.hun in D130306#3670259; that should be addressable in a followup by limiting the CallString depth.

Diff Detail

Event Timeline

samestep created this revision.Jul 26 2022, 11:42 AM
Herald added a project: Restricted Project. · View Herald Transcript
samestep requested review of this revision.Jul 26 2022, 11:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2022, 11:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
samestep edited the summary of this revision. (Show Details)Jul 26 2022, 11:51 AM
samestep updated this revision to Diff 447808.Jul 26 2022, 12:56 PM

Use operator[] instead of try_emplace for C++14 compatibility

While separate call strings are properly isolated from each other, repeated analysis of the same call is not:|

while (...)
{
  foo(...);
}

In the above code snippet, we will end up analyzing foo with leftover state from the previous iteration. The analysis can potentially observe state private to the previous call in the current call. Do I miss something or is this intentional?

samestep added a comment.EditedJul 26 2022, 6:29 PM

While separate call strings are properly isolated from each other, repeated analysis of the same call is not:|

while (...)
{
  foo(...);
}

In the above code snippet, we will end up analyzing foo with leftover state from the previous iteration. The analysis can potentially observe state private to the previous call in the current call. Do I miss something or is this intentional?

Sorry, I originally misread your message and thought you were asking about recursion, not iteration. The answer is yes, this is intentional, in the same way that we use a stable StorageLocation for a variable used inside of a loop.

old message:

Correct, you didn't miss something. I'm not really sure of the best way to deal with this; in the short term, my plan is to only allow analysis of a callee when it is not already present in the call-string. @ymandel any ideas for the longer term?

Note: after some discussion with @ymandel, we've decided to adjust our design for this. A new patch is on the way which will replace this one.

samestep abandoned this revision.Jul 28 2022, 11:54 AM

Here is the new patch: D130726