Skip to content

Commit

Permalink
Fixes to PDG/CDG (#1401)
Browse files Browse the repository at this point in the history 
* Try to remove some confusing edges from PDG

* fixed endless loop in PDG

* now with identityset

* Some more cleanup

---------

Co-authored-by: Alexander Kuechler <[email protected]>
Co-authored-by: Mathias Morbitzer <[email protected]>
  • Loading branch information
@oxisto @KuechA @morbitzer
3 people authored Dec 30, 2023
1 parent d1d86e6 commit 460ad7d
Show file tree
Hide file tree
Showing 4 changed files with 289 additions and 60 deletions.
7 changes: 5 additions & 2 deletions cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/graph/Node.kt
View file
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
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

0 comments on commit 460ad7d

Please sign in to comment.