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.

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

Incorrect topological sorting leads to incorrect SCC computation in Spark #2080

Closed
michael-emmi opened this issue May 15, 2024 · 1 comment · Fixed by #2081
Closed

Incorrect topological sorting leads to incorrect SCC computation in Spark #2080

michael-emmi opened this issue May 15, 2024 · 1 comment · Fixed by #2081

Comments

@michael-emmi
Copy link
Contributor

Please examine each of the following points so that we can help you as soon and best as possible.

Describe the bug

Spark’s SCC-collapsing optimization merges PAG nodes which should be in separate components.

Input file

package soot.jimple.spark.solver;

import java.util.Collections;

import org.junit.Test;

import static org.junit.Assert.assertEquals;

import soot.Scene;
import soot.Type;
import soot.jimple.spark.pag.PAG;
import soot.jimple.spark.pag.VarNode;
import soot.options.SparkOptions;

public class SCCCollapserTest {

    @Test
    public void testSeparateComponents() {
        Scene.v().loadBasicClasses();
        Type type = Scene.v().getObjectType();

        SparkOptions sparkOptions = new SparkOptions(Collections.emptyMap());
        PAG pag = new PAG(sparkOptions);

        VarNode a = pag.makeGlobalVarNode("a", type);
        VarNode b = pag.makeGlobalVarNode("b", type);
        VarNode c = pag.makeGlobalVarNode("c", type);
        pag.addEdge(a, b);
        pag.addEdge(a, c);
        pag.addEdge(b, c);

        SCCCollapser sccCollapser = new SCCCollapser(pag, false);
        sccCollapser.collapse();
        pag.cleanUpMerges();

        assertEquals(a, a.getReplacement());
        assertEquals(b, b.getReplacement());
        assertEquals(c, c.getReplacement());
    }
}

To reproduce

The second assertion fails in the test above.

Expected behavior

The test above should pass, since nodes a, b, and c should each be in their own component.

Stacktrace

N/A

Additional context

It appears that the SCC computation, following Kosaraju’s algorithm, is based on an invalid topological order. The topological sort should order node b before c. Instead, c gets ordered before b. This results in c being the SCC-root for b.

@michael-emmi
Copy link
Contributor Author

This appears to have been broken by #1843

michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 15, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 15, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 15, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 15, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 16, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 16, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 16, 2024
michael-emmi pushed a commit to michael-emmi/soot that referenced this issue May 16, 2024
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 a pull request may close this issue.

1 participant