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

Fixes to PDG/CDG #1401

Merged
merged 4 commits into from
Dec 30, 2023
Merged

Fixes to PDG/CDG #1401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
495dac1
Try to remove some confusing edges from PDG
KuechA Dec 6, 2023
fc96dfc
fixed endless loop in PDG
morbitzer Dec 22, 2023
e636321
now with identityset
morbitzer Dec 27, 2023
ba4ab75
Some more cleanup
oxisto Dec 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/Node.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,11 @@ open class Node : IVisitable<Node>, Persistable, LanguageProvider, ScopeProvider
prev.nextDFGEdges.add(edge)
}

fun addPrevCDG(prev: Node) {
val edge = PropertyEdge(prev, this)
fun addPrevCDG(
prev: Node,
properties: MutableMap<Properties, Any?> = EnumMap(Properties::class.java)
) {
val edge = PropertyEdge(prev, this, properties)
prevCDGEdges.add(edge)
prev.nextCDGEdges.add(edge)
}
Expand Down
217 changes: 171 additions & 46 deletions cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/ControlDependenceGraphPass.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,35 @@ import de.fraunhofer.aisec.cpg.graph.BranchingNode
import de.fraunhofer.aisec.cpg.graph.Node
import de.fraunhofer.aisec.cpg.graph.allChildren
import de.fraunhofer.aisec.cpg.graph.declarations.FunctionDeclaration
import de.fraunhofer.aisec.cpg.graph.declarations.TranslationUnitDeclaration
import de.fraunhofer.aisec.cpg.graph.declarations.cyclomaticComplexity
import de.fraunhofer.aisec.cpg.graph.edge.Properties
import de.fraunhofer.aisec.cpg.graph.edge.PropertyEdge
import de.fraunhofer.aisec.cpg.graph.functions
import de.fraunhofer.aisec.cpg.graph.statements.IfStatement
import de.fraunhofer.aisec.cpg.graph.statements.ReturnStatement
import de.fraunhofer.aisec.cpg.graph.statements.Statement
import de.fraunhofer.aisec.cpg.graph.statements.expressions.ConditionalExpression
import de.fraunhofer.aisec.cpg.graph.statements.expressions.ShortCircuitOperator
import de.fraunhofer.aisec.cpg.helpers.*
import de.fraunhofer.aisec.cpg.passes.order.DependsOn
import java.util.*

/** This pass builds the Control Dependence Graph (CDG) by iterating through the EOG. */
@DependsOn(EvaluationOrderGraphPass::class)
open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnitPass(ctx) {
open class ControlDependenceGraphPass(ctx: TranslationContext) : EOGStarterPass(ctx) {

class Configuration(
/**
* This specifies the maximum complexity (as calculated per
* [Statement.cyclomaticComplexity]) a [FunctionDeclaration] must have in order to be
* considered.
*/
var maxComplexity: Int? = null
) : PassConfiguration()

override fun cleanup() {
// Nothing to do
}

override fun accept(tu: TranslationUnitDeclaration) {
tu.functions.forEach(::handle)
}

/**
* Computes the CDG for the given [functionDeclaration]. It performs the following steps:
* 1) Compute the "parent branching node" for each node and through which path the node is
Expand All @@ -62,19 +70,31 @@ open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnit
* parent node and the path(s) through which the [BranchingNode] node is reachable. 3.c)
* Repeat step 3) until you cannot move the node upwards in the CDG anymore.
*/
private fun handle(functionDeclaration: FunctionDeclaration) {
override fun accept(startNode: Node) {
// For now, we only execute this for function declarations, we will support all EOG starters
// in the future.
if (startNode !is FunctionDeclaration) {
return
}
val max = passConfig<Configuration>()?.maxComplexity
val c = startNode.body?.cyclomaticComplexity ?: 0
if (max != null && c > max) {
log.info(
"Ignoring function ${startNode.name} because its complexity (${c}) is greater than the configured maximum (${max})"
)
return
}

// Maps nodes to their "cdg parent" (i.e. the dominator) and also has the information
// through which path it is reached. If all outgoing paths of the node's dominator result in
// the node, we use the dominator's state instead (i.e., we move the node one layer upwards)
val startState = PrevEOGState()
startState.push(
functionDeclaration,
PrevEOGLattice(mapOf(Pair(functionDeclaration, setOf(functionDeclaration))))
)
val finalState =
iterateEOG(functionDeclaration.nextEOGEdges, startState, ::handleEdge) ?: return
val identityMap = IdentityHashMap<Node, IdentitySet<Node>>()
identityMap[startNode] = identitySetOf(startNode)
startState.push(startNode, PrevEOGLattice(identityMap))
val finalState = iterateEOG(startNode.nextEOGEdges, startState, ::handleEdge) ?: return

val branchingNodeConditionals = getBranchingNodeConditions(functionDeclaration)
val branchingNodeConditionals = getBranchingNodeConditions(startNode)

// Collect the information, identify merge points, etc. This is not really efficient yet :(
for ((node, dominatorPaths) in finalState) {
Expand All @@ -83,15 +103,40 @@ open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnit
.map { (k, v) -> Pair(k, v.toMutableSet()) }
.toMutableList()
val finalDominators = mutableListOf<Pair<Node, MutableSet<Node>>>()
val conditionKeys =
dominatorPaths.elements.entries
.filter { (k, _) ->
(k as? BranchingNode)?.branchedBy == node ||
node in
((k as? BranchingNode)?.branchedBy?.allChildren<Node>() ?: listOf())
}
.map { (k, _) -> k }
if (conditionKeys.isNotEmpty()) {
// The node is part of the condition. For loops, it happens that these nodes are
// somehow put in the CDG of the surrounding statement (e.g. the loop) but we don't
// want this. Move it one layer up.
for (k1 in conditionKeys) {
dominatorsList.removeIf { k1 == it.first }
finalState[k1]?.elements?.forEach { (newK, newV) ->
val entry = dominatorsList.firstOrNull { it.first == newK }
entry?.let {
dominatorsList.remove(entry)
val update = entry.second.addAll(newV)
if (update) dominatorsList.add(entry) else finalDominators.add(entry)
}
?: dominatorsList.add(Pair(newK, newV.toMutableSet()))
}
}
}
val alreadySeen = mutableSetOf<Pair<Node, Set<Node>>>()

while (dominatorsList.isNotEmpty()) {
val (k, v) = dominatorsList.removeFirst()
if (
k != functionDeclaration &&
v.containsAll(branchingNodeConditionals[k] ?: setOf())
) {
// We are reachable from all the branches of branch. Add this parent to the
// worklist or update an existing entry. Also consider already existing entries
// in finalDominators list and update it (if necessary)
alreadySeen.add(Pair(k, v))
if (k != startNode && v.containsAll(branchingNodeConditionals[k] ?: setOf())) {
// We are reachable from all the branches of a branching node. Add this parent
// to the worklist or update an existing entry. Also consider already existing
// entries in finalDominators list and update it (if necessary)
val newDominatorMap = finalState[k]?.elements
newDominatorMap?.forEach { (newK, newV) ->
if (dominatorsList.any { it.first == newK }) {
Expand All @@ -103,10 +148,22 @@ open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnit
val entry = finalDominators.first { it.first == newK }
finalDominators.remove(entry)
val update = entry.second.addAll(newV)
if (update) dominatorsList.add(entry) else finalDominators.add(entry)
} else {
if (
update &&
alreadySeen.none {
it.first == entry.first && it.second == entry.second
}
)
dominatorsList.add(entry)
else finalDominators.add(entry)
} else if (alreadySeen.none { it.first == newK && it.second == newV }) {
// We don't have an entry yet => add a new one
dominatorsList.add(Pair(newK, newV.toMutableSet()))
val newEntry = Pair(newK, newV.toMutableSet())
dominatorsList.add(newEntry)
} else {
// Not sure what to do, there seems to be a cycle but this entry is not
// in finalDominators for some reason. Add to finalDominators now.
finalDominators.add(Pair(newK, newV.toMutableSet()))
}
}
} else {
Expand All @@ -115,9 +172,33 @@ open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnit
finalDominators.add(Pair(k, v))
}
}

// We have all the dominators of this node and potentially traversed the graph
// "upwards". Add the CDG edges
finalDominators.filter { (k, _) -> k != node }.forEach { (k, _) -> node.addPrevCDG(k) }
finalDominators
.filter { (k, _) -> k != node }
.forEach { (k, v) ->
val properties = EnumMap<Properties, Any?>(Properties::class.java)
val branchesSet =
k.nextEOGEdges
.filter { edge -> edge.end in v }
.mapNotNull { it.getProperty(Properties.BRANCH) }
.toSet()
if (branchesSet.size == 1) {
properties[Properties.BRANCH] = branchesSet.single()
} else if (branchesSet.isNotEmpty()) {
properties[Properties.BRANCH] = branchesSet
} else if (
k is IfStatement &&
branchesSet.isEmpty() &&
(branchingNodeConditionals[k]?.size ?: 0) > 1
) {
// The if statement has only a then branch but there's a way to "jump out"
// of this branch. In this case, we want to set the false property here
properties[Properties.BRANCH] = setOf(false)
}
node.addPrevCDG(k, properties)
}
}
}

Expand Down Expand Up @@ -176,19 +257,27 @@ open class ControlDependenceGraphPass(ctx: TranslationContext) : TranslationUnit
*/
fun handleEdge(
currentEdge: PropertyEdge<Node>,
currentState: State<Node, Map<Node, Set<Node>>>,
currentWorklist: Worklist<PropertyEdge<Node>, Node, Map<Node, Set<Node>>>
): State<Node, Map<Node, Set<Node>>> {
currentState: State<Node, IdentityHashMap<Node, IdentitySet<Node>>>,
currentWorklist: Worklist<PropertyEdge<Node>, Node, IdentityHashMap<Node, IdentitySet<Node>>>
): State<Node, IdentityHashMap<Node, IdentitySet<Node>>> {
// Check if we start in a branching node and if this edge leads to the conditional
// branch. In this case, the next node will move "one layer downwards" in the CDG.
if (currentEdge.start is BranchingNode) { // && currentEdge.isConditionalBranch()) {
// We start in a branching node and end in one of the branches, so we have the
// following state:
// for the branching node "start", we have a path through "end".
currentState.push(
currentEdge.end,
PrevEOGLattice(mapOf(Pair(currentEdge.start, setOf(currentEdge.end))))
)
val prevPathLattice =
PrevEOGLattice(
IdentityHashMap(
currentState[currentEdge.start]?.elements?.filter { (k, v) ->
k == currentEdge.start
}
)
)
val map = IdentityHashMap<Node, IdentitySet<Node>>()
map[currentEdge.start] = identitySetOf(currentEdge.end)
val newPath = PrevEOGLattice(map).lub(prevPathLattice) as PrevEOGLattice
currentState.push(currentEdge.end, newPath)
} else {
// We did not start in a branching node, so for the next node, we have the same path
// (last branching + first end node) as for the start node of this edge.
Expand All @@ -198,7 +287,9 @@ fun handleEdge(
val state =
PrevEOGLattice(
currentState[currentEdge.start]?.elements
?: mapOf(Pair(currentEdge.start, setOf(currentEdge.end)))
?: IdentityHashMap(
mutableMapOf(Pair(currentEdge.start, identitySetOf(currentEdge.end)))
)
)
currentState.push(currentEdge.end, state)
}
Expand All @@ -224,36 +315,70 @@ private fun <T : Node> PropertyEdge<T>.isConditionalBranch(): Boolean {
} else
(this.start is IfStatement ||
this.start is ConditionalExpression ||
this.start is ShortCircuitOperator) && this.getProperty(Properties.BRANCH) == false
this.start is ShortCircuitOperator) && this.getProperty(Properties.BRANCH) == false ||
(this.start is IfStatement &&
!(this.start as IfStatement).allBranchesFromMyThenBranchGoThrough(
(this.start as IfStatement).nextUnconditionalNode
))
}

private val IfStatement.nextUnconditionalNode: Node?
get() = this.nextEOGEdges.firstOrNull { it.getProperty(Properties.BRANCH) == null }?.end

private fun IfStatement.allBranchesFromMyThenBranchGoThrough(node: Node?): Boolean {
if (this.thenStatement.allChildren<ReturnStatement>().isNotEmpty()) return false

if (node == null) return true

val alreadySeen = mutableSetOf<Node>()
val nextNodes =
this.nextEOGEdges
.filter { it.getProperty(Properties.BRANCH) == true }
.map { it.end }
.toMutableList()

while (nextNodes.isNotEmpty()) {
val nextNode = nextNodes.removeFirst()
if (nextNode == node) {
continue
} else if (nextNode.nextEOG.isEmpty()) {
// We're at the end of the EOG but didn't see "node" on this path. Fail
return false
}
alreadySeen.add(nextNode)
nextNodes.addAll(nextNode.nextEOG.filter { it !in alreadySeen })
}

return true
}

/**
* Implements the [LatticeElement] over a set of nodes and their set of "nextEOG" nodes which reach
* this node.
*/
class PrevEOGLattice(override val elements: Map<Node, Set<Node>>) :
LatticeElement<Map<Node, Set<Node>>>(elements) {
class PrevEOGLattice(override val elements: IdentityHashMap<Node, IdentitySet<Node>>) :
LatticeElement<IdentityHashMap<Node, IdentitySet<Node>>>(elements) {

override fun lub(
other: LatticeElement<Map<Node, Set<Node>>>
): LatticeElement<Map<Node, Set<Node>>> {
val newMap = (other.elements).mapValues { (_, v) -> v.toMutableSet() }.toMutableMap()
other: LatticeElement<IdentityHashMap<Node, IdentitySet<Node>>>
): LatticeElement<IdentityHashMap<Node, IdentitySet<Node>>> {
val newMap = IdentityHashMap(other.elements.mapValues { (_, v) -> v.toIdentitySet() })
for ((key, value) in this.elements) {
newMap.computeIfAbsent(key, ::mutableSetOf).addAll(value)
newMap.computeIfAbsent(key, ::identitySetOf).addAll(value)
}
return PrevEOGLattice(newMap)
}

override fun duplicate() = PrevEOGLattice(this.elements.toMap())
override fun duplicate() = PrevEOGLattice(IdentityHashMap(this.elements))

override fun compareTo(other: LatticeElement<Map<Node, Set<Node>>>): Int {
override fun compareTo(other: LatticeElement<IdentityHashMap<Node, IdentitySet<Node>>>): Int {
return if (
this.elements.keys.containsAll(other.elements.keys) &&
this.elements.all { (k, v) -> v.containsAll(other.elements[k] ?: setOf()) }
this.elements.all { (k, v) -> v.containsAll(other.elements[k] ?: identitySetOf()) }
) {
if (
this.elements.keys.size > (other.elements.keys.size) ||
this.elements.any { (k, v) -> v.size > (other.elements[k] ?: setOf()).size }
this.elements.any { (k, v) -> v.size > (other.elements[k]?.size ?: 0) }
)
1
else 0
Expand All @@ -267,4 +392,4 @@ class PrevEOGLattice(override val elements: Map<Node, Set<Node>>) :
* A state which actually holds a state for all [PropertyEdge]s. It maps the node to its
* [BranchingNode]-parent and the path through which it is reached.
*/
class PrevEOGState : State<Node, Map<Node, Set<Node>>>()
class PrevEOGState : State<Node, IdentityHashMap<Node, IdentitySet<Node>>>()
Loading
Loading