Page MenuHomePhabricator

[clang][dataflow] Add an option for context-sensitive depth
ClosedPublic

Authored by samestep on Aug 12 2022, 2:18 PM.

Details

Summary

This patch adds a Depth field (default value 2) to ContextSensitiveOptions, allowing context-sensitive analysis of functions that call other functions. This also requires replacing the DeclCtx field on Environment with a CallString field that contains a vector of decl contexts, to ensure that the analysis doesn't try to analyze recursive or mutually recursive calls (which would result in a crash, due to the way we handle StorageLocations).

Diff Detail

Event Timeline

samestep created this revision.Aug 12 2022, 2:18 PM
Herald added a project: Restricted Project. · View Herald Transcript
samestep requested review of this revision.Aug 12 2022, 2:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2022, 2:18 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
samestep edited the summary of this revision. (Show Details)Aug 12 2022, 2:20 PM
samestep added reviewers: sgatev, gribozavr2, xazax.hun, wyt.
samestep edited the summary of this revision. (Show Details)Aug 12 2022, 2:21 PM
sgatev added inline comments.Aug 15 2022, 7:49 AM
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
395
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
210

If canDescend is supposed to return false for MaxDepth = 0, shouldn't this be <?

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
4033

Let's add a similar test with Depth set to 0.

xazax.hun accepted this revision.Aug 15 2022, 8:22 AM

Once the Stanislav's comments are resolved, it looks good to me.

clang/lib/Analysis/FlowSensitive/Transfer.cpp
665

Alternatively, canDescend could get the optional ContextSensitiveOpts and we can do all the checking there.

This revision is now accepted and ready to land.Aug 15 2022, 8:22 AM
samestep added inline comments.Aug 15 2022, 12:50 PM
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
210

I don't follow; could you clarify? The CallStack should always be nonempty.

clang/lib/Analysis/FlowSensitive/Transfer.cpp
665

Ah, good idea!

samestep added inline comments.Aug 15 2022, 12:54 PM
clang/lib/Analysis/FlowSensitive/Transfer.cpp
665

Oh... actually that doesn't work quite so nicely, because it introduces a circular dependency between Transfer.h and DataflowEnvironment.h. I'll leave it as-is for now.

samestep updated this revision to Diff 452783.Aug 15 2022, 12:55 PM

Address Stanislav's comments

This revision was landed with ongoing or failed builds.Aug 15 2022, 12:58 PM
This revision was automatically updated to reflect the committed changes.