Skip to content

Adding soft from DynamicInvokeResolver to ControlFlowSensitiveDFGPass #1532

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.

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

Merged
merged 3 commits into from
Apr 17, 2024

Conversation

konradweiss
Copy link
Collaborator

Solves #1531

@konradweiss
Copy link
Collaborator Author

Apparently there is now a problem with dependency ordering between passes. We have to solve that before this can be merged.

@oxisto
Copy link
Member

oxisto commented Apr 16, 2024

Apparently there is now a problem with dependency ordering between passes. We have to solve that before this can be merged.

Hm yes it looks like the dependency matching algorithm is aborting because the ControlFlowSensitiveDFGPass is not registered in the test; but you explicitly added it as a "soft" dependency, so looks like this is broken :(

@oxisto
Copy link
Member

oxisto commented Apr 16, 2024

Can you try with the following patch?

diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDependencies.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDependencies.kt
index 4ee5f627d..50ff80b02 100644
--- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDependencies.kt
+++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDependencies.kt
@@ -64,4 +64,32 @@ data class PassWithDependencies(
         }
         return builder.toString()
     }
+
+    /**
+     * Checks whether the [dependencies] of this pass are met. The list of [softDependencies] and
+     * [hardDependencies] is removed step-by-step in
+     * [PassWithDepsContainer.getAndRemoveFirstPassWithoutDependencies].
+     */
+    fun dependenciesMet(workingList: MutableList<PassWithDependencies>): Boolean {
+        // In the simplest case all our dependencies are empty since they were already removed by
+        // the selecting algorithm.
+        if (this.dependencies.isEmpty() && !this.isLastPass) {
+            return true
+        }
+
+        // We also need to check, whether we still "soft" depend on passes that are just not
+        // there (after all hard dependencies are met), in this case we can select the pass
+        // as well
+        val remainingClasses = workingList.map { it.pass }
+        if (
+            this.hardDependencies.isEmpty() &&
+                this.softDependencies.all { !remainingClasses.contains(it) } &&
+                !this.isLastPass
+        ) {
+            return true
+        }
+
+        // Otherwise, we still depend on an unselected pass
+        return false
+    }
 }
diff --git a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDepsContainer.kt b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDepsContainer.kt
index e6b5988fb..c3219e55c 100644
--- a/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDepsContainer.kt
+++ b/cpg-core/src/main/kotlin/de/fraunhofer/aisec/cpg/passes/configuration/PassWithDepsContainer.kt
@@ -160,9 +160,8 @@ class PassWithDepsContainer {
 
             // Keep going until our dependencies are met, this will collect passes that can run in
             // parallel in results
-            if (
-                currentElement.dependencies.isEmpty() && !currentElement.isLastPass
-            ) { // no unsatisfied dependencies
+            if (currentElement.dependenciesMet(workingList)) {
+                // no unsatisfied dependencies
                 val result = currentElement.pass
                 results.add(result)

konradweiss and others added 3 commits April 17, 2024 10:04

Verified

This commit was signed with the committer’s verified signature. The key has expired.
NeQuissimus Tim Steinbach
…tiveDFGPass

Verified

This commit was signed with the committer’s verified signature. The key has expired.
NeQuissimus Tim Steinbach

Verified

This commit was signed with the committer’s verified signature. The key has expired.
NeQuissimus Tim Steinbach
…cInvokeResolver.kt

Co-authored-by: Christian Banse <christian.banse@aisec.fraunhofer.de>
@konradweiss konradweiss force-pushed the dynamicinvokes-depend-cfsdfg branch from 151eb22 to 2219f0b Compare April 17, 2024 08:04
Copy link

@oxisto oxisto merged commit 93cf6e0 into main Apr 17, 2024
3 checks passed
@oxisto oxisto deleted the dynamicinvokes-depend-cfsdfg branch April 17, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants