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

update implementation for BackwardsStmtGraph and add PostDominanceFinder utility #921

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

shenjunjiekoda
Copy link
Contributor

@shenjunjiekoda shenjunjiekoda commented Apr 13, 2024

update the implementation for BackwardsStmtGraph in sootup.core.graph

before in DominanceFinder.java:

List<BasicBlock<?>> preds = new ArrayList<>(block.getPredecessors());

after in DominanceFinder.java:

List<BasicBlock<?>> preds = new ArrayList<>(blockGraph.predecessors(block));

Apply this change , we can create PostDominaceFinder as a wrapper of DominanceFinder directly through use backwardStmtGraph as internal backingGraph. To implement this, I add successors/predecessors api in StmtGraph and its subclasses which use block as param instead of dependending on the BasicBlock api: getPredecessors/getSuccessors.

public class PostDominanceFinder extends DominanceFinder {

  public PostDominanceFinder(StmtGraph<?> blockGraph) {
    super(new BackwardsStmtGraph(blockGraph));
  }
}

Thank you for considering this update!

@shenjunjiekoda shenjunjiekoda changed the title update implementation for BackwardsStmtGraph and add PostDominanceFinder update implementation for BackwardsStmtGraph Apr 14, 2024
@shenjunjiekoda shenjunjiekoda changed the title update implementation for BackwardsStmtGraph update implementation for BackwardsStmtGraph and add PostDominanceFinder utility Apr 15, 2024
@JonasKlauke JonasKlauke requested a review from kadirayk April 15, 2024 08:16
Copy link
Collaborator

@swissiety swissiety left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
We are trying to introduce only what is really necessary to the API to not run into Soot problems with SootUp - I think adding all those successor/predecessor methods pollutes the API a bit. How about adding a method inside the Post- and DominanceFinder to initialize the predecessors they way they are needed by the respective *DominanceFinder - i think that would be a simpler & cleaner approach :)

Copy link
Collaborator

@swissiety swissiety left a comment

Choose a reason for hiding this comment

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

looks good 👍

Copy link
Collaborator

@swissiety swissiety left a comment

Choose a reason for hiding this comment

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

good job!

@swissiety swissiety merged commit 9448ed9 into soot-oss:develop Apr 15, 2024
7 checks passed
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.

2 participants