#135 create dummies for all calls whose targets are assumed to be in …

…known RecordDeclarations, fix tests to adapt to this behavior
Fraunhofer-AISEC · Aug 17, 2020 · 22bd52c · 22bd52c
1 parent 61f4253
commit 22bd52c
Show file tree

Hide file tree

Showing 10 changed files with 71 additions and 37 deletions.
diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/FunctionDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/FunctionDeclaration.java
@@ -28,11 +28,7 @@
 
 import de.fraunhofer.aisec.cpg.graph.type.Type;
 import de.fraunhofer.aisec.cpg.graph.type.UnknownType;
-import java.util.ArrayList;
-import java.util.Comparator;
-import java.util.List;
-import java.util.Objects;
-import java.util.Optional;
+import java.util.*;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -221,7 +217,11 @@ public String toString() {
     return new ToStringBuilder(this, Node.TO_STRING_STYLE)
         .appendSuper(super.toString())
         .append("type", type)
-        .append("parameters", parameters.stream().map(ParamVariableDeclaration::getName))
+        .append(
+            "parameters",
+            parameters.stream()
+                .map(ParamVariableDeclaration::getName)
+                .collect(Collectors.joining(", ")))
         .toString();
   }
 

diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java
@@ -285,6 +285,10 @@ private void handleNormalCalls(RecordDeclaration curClass, CallExpression call)
               .filter(
                   f -> f.getName().equals(call.getName()) && f.hasSignature(call.getSignature()))
               .collect(Collectors.toList());
+      if (invocationCandidates.isEmpty()) {
+        invocationCandidates =
+            List.of(createDummy(null, call.getName(), call.getCode(), false, call.getSignature()));
+      }
 
       call.setInvokes(invocationCandidates);
     } else if (!handlePossibleStaticImport(call, curClass)) {
@@ -295,13 +299,24 @@ private void handleNormalCalls(RecordDeclaration curClass, CallExpression call)
   private void handleMethodCall(RecordDeclaration curClass, CallExpression call) {
     Set<Type> possibleContainingTypes = getPossibleContainingTypes(call, curClass);
 
-    // Find invokes by type
+    // Find overridden invokes
     List<FunctionDeclaration> invocationCandidates =
         call.getInvokes().stream()
             .map(f -> getOverridingCandidates(possibleContainingTypes, f))
             .flatMap(Collection::stream)
             .collect(Collectors.toList());
 
+    // Find function targets
+    if (invocationCandidates.isEmpty() && currentTU != null) {
+      invocationCandidates =
+          currentTU.getDeclarations().stream()
+              .filter(FunctionDeclaration.class::isInstance)
+              .map(FunctionDeclaration.class::cast)
+              .filter(
+                  f -> f.getName().equals(call.getName()) && f.hasSignature(call.getSignature()))
+              .collect(Collectors.toList());
+    }
+
     // Find invokes by supertypes
     if (invocationCandidates.isEmpty()) {
       String[] nameParts = call.getName().split("\\.");
@@ -321,6 +336,14 @@ private void handleMethodCall(RecordDeclaration curClass, CallExpression call) {
         && !(call instanceof MemberCallExpression || call instanceof StaticCallExpression)) {
       call.setBase(curClass.getThis());
     }
+
+    if (invocationCandidates.isEmpty()) {
+      possibleContainingTypes.stream()
+          .map(t -> recordMap.get(t.getTypeName()))
+          .filter(Objects::nonNull)
+          .map(r -> createDummy(r, call.getName(), call.getCode(), false, call.getSignature()))
+          .forEach(invocationCandidates::add);
+    }
     call.setInvokes(invocationCandidates);
   }
 
@@ -445,7 +468,8 @@ private void generateStaticImportDummies(
     }
   }
 
-  private FunctionDeclaration createDummyWithMatchingSignature(
+  @NonNull
+  private FunctionDeclaration createDummy(
       RecordDeclaration containingRecord,
       String name,
       String code,
@@ -471,7 +495,7 @@ private FunctionDeclaration createDummyWithMatchingSignature(
             "No current translation unit when trying to generate function dummy {}",
             dummy.getName());
       } else {
-        currentTU.getDeclarations().add(dummy);
+        currentTU.add(dummy);
       }
       return dummy;
     }
@@ -503,7 +527,6 @@ private Set<Type> getPossibleContainingTypes(Node node, RecordDeclaration curCla
       }
     } else if (curClass != null) {
       possibleTypes.add(TypeParser.createFrom(curClass.getName(), true));
-      possibleTypes.addAll(curClass.getSuperTypes());
     }
     return possibleTypes;
   }

diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/JavaVsCppTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/JavaVsCppTest.java
@@ -60,7 +60,7 @@ private void analyzeAndSave(String pathname) throws Exception {
     assertTrue(decl instanceof RecordDeclaration);
     RecordDeclaration rec = (RecordDeclaration) decl;
     assertEquals("Simple", rec.getName());
-    assertEquals(1, rec.getMethods().size());
+    assertEquals(2, rec.getMethods().size());  // printf dummy
     assertEquals("class", rec.getKind());
 
     MethodDeclaration methodDeclaration = rec.getMethods().get(0);

diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/StaticImportsTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/StaticImportsTest.java
@@ -76,6 +76,10 @@ void testAsteriskImport() throws Exception {
         TestUtils.analyze("java", topLevel.resolve("asterisk"), true);
     List<MethodDeclaration> methods = TestUtils.subnodesOfType(result, MethodDeclaration.class);
     MethodDeclaration main = TestUtils.findByUniqueName(methods, "main");
+    List<RecordDeclaration> records = TestUtils.subnodesOfType(result, RecordDeclaration.class);
+    RecordDeclaration A = TestUtils.findByUniqueName(records, "A");
+    RecordDeclaration B = TestUtils.findByUniqueName(records, "B");
+
     for (CallExpression call : TestUtils.subnodesOfType(main, CallExpression.class)) {
       switch (call.getName()) {
         case "a":
@@ -94,12 +98,12 @@ void testAsteriskImport() throws Exception {
                   .collect(Collectors.toList()));
           break;
         case "nonStatic":
-          assertTrue(call.getInvokes().isEmpty());
+          MethodDeclaration nonStatic = TestUtils.findByUniqueName(B.getMethods(), "nonStatic");
+          assertTrue(nonStatic.isImplicit());
+          assertEquals(List.of(nonStatic), call.getInvokes());
       }
     }
 
-    List<RecordDeclaration> records = TestUtils.subnodesOfType(result, RecordDeclaration.class);
-    RecordDeclaration A = TestUtils.findByUniqueName(records, "A");
     List<FieldDeclaration> testFields = TestUtils.subnodesOfType(A, FieldDeclaration.class);
     FieldDeclaration staticField = TestUtils.findByUniqueName(testFields, "staticField");
     FieldDeclaration nonStaticField = TestUtils.findByUniqueName(testFields, "nonStaticField");

diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/CallResolverTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/CallResolverTest.java
@@ -57,6 +57,12 @@ private void testMethods(List<RecordDeclaration> records, Type intType, Type str
     List<MethodDeclaration> superMethods =
         TestUtils.findByName(
             TestUtils.subnodesOfType(superClassRecord, MethodDeclaration.class), "superTarget");
+    // We can't infer that a call to superTarget(int, int, int) is intended to be part of the
+    // superclass. It looks like a call to a member of Calls.java, thus we need to add these
+    // methods to the lookup
+    superMethods.addAll(
+        TestUtils.findByName(
+            TestUtils.subnodesOfType(callsRecord, MethodDeclaration.class), "superTarget"));
     List<CallExpression> superCalls =
         TestUtils.findByName(
             TestUtils.subnodesOfType(callsRecord, CallExpression.class), "superTarget");
@@ -88,21 +94,23 @@ private void checkCalls(
     List<List<Type>> signatures =
         List.of(List.of(), List.of(intType, intType), List.of(intType, stringType));
     for (List<Type> signature : signatures) {
-      CallExpression call =
-          TestUtils.findByUniquePredicate(calls, c -> c.getSignature().equals(signature));
-      FunctionDeclaration target =
-          TestUtils.findByUniquePredicate(methods, m -> m.hasSignature(signature));
-      assertEquals(List.of(target), call.getInvokes());
+      for (CallExpression call :
+          TestUtils.findByPredicate(calls, c -> c.getSignature().equals(signature))) {
+        FunctionDeclaration target =
+            TestUtils.findByUniquePredicate(methods, m -> m.hasSignature(signature));
+        assertEquals(List.of(target), call.getInvokes());
+      }
     }
 
     // Check for dummies
     List<Type> dummySignature = List.of(intType, intType, intType);
-    CallExpression dummyCall =
-        TestUtils.findByUniquePredicate(calls, c -> c.getSignature().equals(dummySignature));
-    FunctionDeclaration dummyTarget =
-        TestUtils.findByUniquePredicate(methods, m -> m.hasSignature(dummySignature));
-    assertEquals(List.of(dummyTarget), dummyCall.getInvokes());
-    assertTrue(dummyTarget.isImplicit());
+    for (CallExpression dummyCall :
+        TestUtils.findByPredicate(calls, c -> c.getSignature().equals(dummySignature))) {
+      FunctionDeclaration dummyTarget =
+          TestUtils.findByUniquePredicate(methods, m -> m.hasSignature(dummySignature));
+      assertEquals(List.of(dummyTarget), dummyCall.getInvokes());
+      assertTrue(dummyTarget.isImplicit());
+    }
   }
 
   @Test
@@ -129,7 +137,7 @@ void testCpp() throws Exception {
     List<FunctionDeclaration> functions =
         TestUtils.findByPredicate(
             TestUtils.subnodesOfType(result, FunctionDeclaration.class),
-            f -> !(f instanceof MethodDeclaration));
+            f -> f.getName().equals("functionTarget") && !(f instanceof MethodDeclaration));
     List<CallExpression> calls =
         TestUtils.findByName(
             TestUtils.subnodesOfType(result, CallExpression.class), "functionTarget");

diff --git a/src/test/resources/calls/Calls.java b/src/test/resources/calls/Calls.java
@@ -27,8 +27,6 @@ public static void main(String[] args) {
     e.superTarget();
     e.superTarget(1, 2);
     e.superTarget(1, "2");
-    // dummy
-    e.superTarget(1, 2, 3);
 
     Unknown u = new Unknown();
     // don't create dummy for methods of unknown classes!

diff --git a/src/test/resources/calls/External.java b/src/test/resources/calls/External.java
@@ -2,5 +2,5 @@ public class External extends SuperClass {
 
   public void externalTarget() {}
   public void externalTarget(int param1, int param2) {}
-  public void externalTarget(int param1, int param2) {}
+  public void externalTarget(int param1, String param2) {}
 }
diff --git a/src/test/resources/calls/calls.cpp b/src/test/resources/calls/calls.cpp
@@ -26,8 +26,6 @@ class Calls: SuperClass {
       functionTarget();
       functionTarget(1, 2);
       functionTarget(1, "2");
-      // dummy
-      functionTarget(1, 2, 3);
 
       innerTarget();
       innerTarget(1, 2);
@@ -51,11 +49,14 @@ class Calls: SuperClass {
       e.superTarget();
       e.superTarget(1, 2);
       e.superTarget(1, "2");
-      // dummy
-      e.superTarget(1, 2, 3);
 
       Unknown u;
       // don't create dummy for methods of unknown classes!
       u.unknownTarget();
     }
-};
+};
+
+void main() {
+  // dummy
+  functionTarget(1, 2, 3);
+}
diff --git a/src/test/resources/functiondecl.cpp b/src/test/resources/functiondecl.cpp
@@ -8,7 +8,7 @@ int function1(int arg0, std::string arg1, SomeType* arg2, AnotherType &arg3) {
 
 // body for the function declared earlier. should connect the body to the original declaration
 void function0(int arg0) {
-  callSomething();
+  function2();
   // no explicit return
 }
 

diff --git a/src/test/resources/staticImports/asterisk/B.java b/src/test/resources/staticImports/asterisk/B.java
@@ -6,8 +6,8 @@ public static void main(String[] args) {
     a();
     b();
     b(true);
-    nonStatic(); // needs to stay unresolved
+    nonStatic(); // must not be resolved to A.nonStatic but rather a dummy in B
     int y = staticField;
-    int z = nonStaticField; // needs to stay unresolved
+    int z = nonStaticField; // must not be resolved to A.nonStaticField but rather a dummy in B
   }
 }