From 22bd52c44a303a8edf119b11f1aa1c747683e4f5 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Mon, 17 Aug 2020 16:08:19 +0200 Subject: [PATCH] #135 create dummies for all calls whose targets are assumed to be in known RecordDeclarations, fix tests to adapt to this behavior --- .../aisec/cpg/graph/FunctionDeclaration.java | 12 +++---- .../aisec/cpg/passes/CallResolver.java | 31 +++++++++++++++--- .../aisec/cpg/enhancements/JavaVsCppTest.java | 2 +- .../cpg/enhancements/StaticImportsTest.java | 10 ++++-- .../enhancements/calls/CallResolverTest.java | 32 ++++++++++++------- src/test/resources/calls/Calls.java | 2 -- src/test/resources/calls/External.java | 2 +- src/test/resources/calls/calls.cpp | 11 ++++--- src/test/resources/functiondecl.cpp | 2 +- .../resources/staticImports/asterisk/B.java | 4 +-- 10 files changed, 71 insertions(+), 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 index e48464541d..f0299d352b 100644 --- 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 index e0d22db260..05b158cead 100644 --- 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 possibleContainingTypes = getPossibleContainingTypes(call, curClass); - // Find invokes by type + // Find overridden invokes List 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 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 index eade946031..e2c77911d9 100644 --- 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 index f27b4c07ff..c319715a1e 100644 --- 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 methods = TestUtils.subnodesOfType(result, MethodDeclaration.class); MethodDeclaration main = TestUtils.findByUniqueName(methods, "main"); + List 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 records = TestUtils.subnodesOfType(result, RecordDeclaration.class); - RecordDeclaration A = TestUtils.findByUniqueName(records, "A"); List 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 index efabdd9aa9..35e32f0af3 100644 --- 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 records, Type intType, Type str List 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 superCalls = TestUtils.findByName( TestUtils.subnodesOfType(callsRecord, CallExpression.class), "superTarget"); @@ -88,21 +94,23 @@ private void checkCalls( List> signatures = List.of(List.of(), List.of(intType, intType), List.of(intType, stringType)); for (List 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 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 functions = TestUtils.findByPredicate( TestUtils.subnodesOfType(result, FunctionDeclaration.class), - f -> !(f instanceof MethodDeclaration)); + f -> f.getName().equals("functionTarget") && !(f instanceof MethodDeclaration)); List 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 index 5924540c6f..eb8e63a6f4 100644 --- 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 index e488114464..59381c901f 100644 --- 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) {} } \ No newline at end of file diff --git a/src/test/resources/calls/calls.cpp b/src/test/resources/calls/calls.cpp index 05638ec3fe..47dd8a6690 100644 --- 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(); } -}; \ No newline at end of file +}; + +void main() { + // dummy + functionTarget(1, 2, 3); +} \ No newline at end of file diff --git a/src/test/resources/functiondecl.cpp b/src/test/resources/functiondecl.cpp index 31ce0c08c9..0f69803ff7 100644 --- 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 index d3c9275aea..888c38d9df 100644 --- 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 } } \ No newline at end of file