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

Fix CallGraphAlgorithm.addClass(..) implementation #783

Merged
merged 3 commits into from
Dec 21, 2023
Merged

Fix CallGraphAlgorithm.addClass(..) implementation #783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
023e566
fix IAE:Node not found from vertexOf(..) via .callsTo(..)
swissiety Dec 21, 2023
011a239
make API return type more convenient
swissiety Dec 21, 2023
54191f1
not throwing an error if not necessary when nothing was added to the …
swissiety Dec 21, 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
21 changes: 14 additions & 7 deletions sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,16 +313,21 @@
@Nonnull
@Override
public CallGraph addClass(@Nonnull CallGraph oldCallGraph, @Nonnull JavaClassType classType) {
MutableCallGraph updated = oldCallGraph.copy();

SootClass<?> clazz = view.getClassOrThrow(classType);
Set<MethodSignature> newMethodSignatures =
clazz.getMethods().stream().map(Method::getSignature).collect(Collectors.toSet());
clazz.getMethods().stream()
.map(Method::getSignature)
.filter(methodSig -> !oldCallGraph.containsMethod(methodSig))
.collect(Collectors.toSet());

if (newMethodSignatures.stream().anyMatch(oldCallGraph::containsMethod)) {
throw new IllegalArgumentException("CallGraph already contains methods from " + classType);
// were all the added method signatures already visited in the CallGraph? i.e. is there
// something to add?
if (newMethodSignatures.isEmpty()) {
return oldCallGraph;

Check warning on line 326 in sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java

View check run for this annotation

Codecov / codecov/patch

sootup.callgraph/src/main/java/sootup/callgraph/AbstractCallGraphAlgorithm.java#L326 

Added line #L326 was not covered by tests
}

MutableCallGraph updated = oldCallGraph.copy();

// Step 1: Add edges from the new methods to other methods
Deque<MethodSignature> workList = new ArrayDeque<>(newMethodSignatures);
Set<MethodSignature> processed = new HashSet<>(oldCallGraph.getMethodSignatures());
Expand Down Expand Up @@ -354,8 +359,10 @@
MethodSignature overridingMethodSig =
clazz.getMethod(overriddenMethodSig.getSubSignature()).get().getSignature();

for (MethodSignature callingMethodSig : updated.callsTo(overriddenMethodSig)) {
updated.addCall(callingMethodSig, overridingMethodSig);
if (updated.containsMethod(overriddenMethodSig)) {
for (MethodSignature callingMethodSig : updated.callsTo(overriddenMethodSig)) {
updated.addCall(callingMethodSig, overridingMethodSig);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is only true for CHA algorithm or for the RTA (if the given classtype is instantiated)
If we want to actually support this function the abstaractcallgraphalgorithm has to be abstract and this method has to be developed for each algorithm.

additionally if this class was already in the view all edges are already included.
I think the only reason for this method if you want to force the call graph to contain the class, even when the algorithm would say it is not contained, and then we could do it like this (like CHA)

}
}
});

Expand Down
7 changes: 7 additions & 0 deletions sootup.java.core/src/main/java/sootup/java/core/JavaSootClass.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import sootup.core.signatures.MethodSubSignature;
import sootup.core.types.ClassType;
import sootup.core.types.Type;
import sootup.java.core.types.JavaClassType;
import sootup.java.core.views.JavaView;

public class JavaSootClass extends SootClass<JavaSootClassSource> {
Expand All @@ -45,6 +46,12 @@ public JavaSootClass(JavaSootClassSource classSource, SourceType sourceType) {
super(classSource, sourceType);
}

@Nonnull
@Override
public JavaClassType getType() {
return (JavaClassType) super.getType();
}

/**
* Get all annotations on this class. If provided with a View, will also resolve all inherited
* annotations from super classes.
Expand Down