Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improvements to EOG iteration and more applications #1135

Merged
merged 49 commits into from
Jul 26, 2023
Merged

Improvements to EOG iteration and more applications #1135

merged 49 commits into from
Jul 26, 2023

Conversation

KuechA
Copy link
Contributor

@KuechA KuechA commented Mar 18, 2023 •
edited
Loading

Multiple (future) passes or analyses compute a fix-point by traversing the EOG and performing some operations. We do not want to implement the same strategy multiple times, so I tried to generalize the EOG iteration and the respective transformation of a state.

This PR...

  • adds a (basic) implementation of the "worklist" fix-point iteration and provides generic interface for lattices and the program state.
  • changes the ControlFlowSensitiveDFGPass to make use of this algorithm
  • handles InitializerListExpression (closes Missing DataFlow Edge for InitializerListExpression #1141 but turned out to be a bug in the frontend)
  • adds a new node abstracting all kinds of nodes which have a conditional execution of paths (BranchingNode) as discussed in (PDG for Branch nodes #1066). It has the field branchingDecision which is the condition or similar. The following nodes implement this interface:
    • IfStatement
    • WhileStatement
    • ForStatement
    • ForEachStatement
    • SwitchStatement
    • CatchClause (even if it is incorrectly handled in the EOGPass)
    • ConditionalExpression (see Missing DataFlow edge for ConditionalExpression #1142 )
    • ShortCircuitOperator which extends BinaryOperator. This one has to be adapted for each language (i.e., the language frontend has to decide if a binary op is a normal BinaryOperator or a ShortCircuitOperator. There's already a configuration in the Language class).
  • adds the fields nextCDGEdges and prevCDGEdges to the class Node. These hold the control dependence graph.
  • implements the new (optional?) pass ControlDependenceGraphPass which adds the CDG edges for each function (intraprocedurally).
  • Propagates the Unreachable properties of EOG edges (Closes Propagate UNREACHABLE Property for EOG edges #1067)

@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

80.5% 80.5% Coverage
6.7% 6.7% Duplication

@KuechA KuechA mentioned this pull request Mar 27, 2023
Open
@KuechA
Copy link
Contributor Author

KuechA commented May 23, 2023

@peckto Could you test if there are performance issues with the new and redesigned passes which haven't been there before?

@KuechA KuechA marked this pull request as ready for review May 30, 2023 07:25
oxisto
oxisto reviewed May 30, 2023
View reviewed changes
cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/UnreachableEOGPass.kt Outdated Show resolved Hide resolved
cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/UnreachableEOGPass.kt Outdated Show resolved Hide resolved
cpg-analysis/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/UnreachableEOGPass.kt Outdated Show resolved Hide resolved
cpg-analysis/src/test/kotlin/de/fraunhofer/aisec/cpg/analysis/MultiValueEvaluatorTest.kt Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/frontends/LanguageTraits.kt Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlDependenceGraphPass.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlDependenceGraphPass.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlDependenceGraphPass.kt Outdated Show resolved Hide resolved
oxisto
oxisto reviewed Jun 15, 2023
View reviewed changes
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/helpers/EOGWorklist.kt Outdated Show resolved Hide resolved
cpg-core/src/test/resources/dfg/initializerListExpression.cpp Outdated Show resolved Hide resolved
cpg-analysis/src/test/kotlin/de/fraunhofer/aisec/cpg/analysis/MultiValueEvaluatorTest.kt Show resolved Hide resolved
oxisto
oxisto reviewed Jun 15, 2023
View reviewed changes
cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlFlowSensitiveDFGPass.kt Outdated
/** A mapping of a [Node] to its [Lattice]. */
var generalState: State<Node, V> = State(),
/** A mapping of [Declaration] to its [Lattice]. */
var declarationsState: State<Node, V> = State(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The declaration state seems to contain more than just declarations - is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is because the "generalState" collects the nodes which will later receive DFG edges whereas the nodes here don't. I documented this but we can also change the name if necessary.

oxisto
oxisto approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@oxisto oxisto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the conflict is resolved

@sonarcloud
Copy link

sonarcloud bot commented Jul 26, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

80.0% 80.0% Coverage
3.1% 3.1% Duplication

@oxisto oxisto enabled auto-merge (squash) July 26, 2023 07:49
@oxisto oxisto dismissed konradweiss’s stale review July 26, 2023 07:50

Comments were addressed

@oxisto oxisto merged commit 066abcf into main Jul 26, 2023
3 checks passed
@oxisto oxisto deleted the worklist-eog-iteration branch July 26, 2023 07:50
oxisto pushed a commit that referenced this pull request Jul 27, 2023
@KuechA @oxisto
Improvements to EOG iteration and more applications (#1135)
80e5b9e
oxisto pushed a commit that referenced this pull request Jul 27, 2023
@KuechA @oxisto
Improvements to EOG iteration and more applications (#1135)
564bbe6
oxisto pushed a commit that referenced this pull request Jul 27, 2023
@KuechA @oxisto
Improvements to EOG iteration and more applications (#1135)
f5a9739
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seelchen seelchen seelchen left review comments

@oxisto oxisto oxisto approved these changes

@konradweiss konradweiss konradweiss left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing DataFlow Edge for InitializerListExpression Propagate UNREACHABLE Property for EOG edges
4 participants
@KuechA @oxisto @konradweiss @seelchen