From 762e6017865d93ecb9035449765bb092bab0ab46 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 13 Aug 2020 10:40:06 +0200 Subject: [PATCH 1/5] Fix NPE when analyzing zero files --- .../aisec/cpg/passes/JavaExternalTypeHierarchyResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.java index b003628c3a..e08833fe49 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/JavaExternalTypeHierarchyResolver.java @@ -22,7 +22,7 @@ public class JavaExternalTypeHierarchyResolver extends Pass { @Override public void accept(TranslationResult translationResult) { // Run only for Java. - if (JavaLanguageFrontend.class.equals(this.lang.getClass())) { + if (this.lang instanceof JavaLanguageFrontend) { TypeSolver resolver = ((JavaLanguageFrontend) this.lang).getNativeTypeResolver(); TypeManager tm = TypeManager.getInstance(); From df0c644c3776a5ac58bb5d6a8764ec60af7901b4 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 13 Aug 2020 14:39:52 +0200 Subject: [PATCH 2/5] Add test for call resolver, improve predicate finding in test utils --- .../de/fraunhofer/aisec/cpg/graph/Node.java | 2 + .../de/fraunhofer/aisec/cpg/TestUtils.java | 18 ++- .../aisec/cpg/enhancements/EOGTest.java | 2 +- .../enhancements/calls/CallResolverTest.java | 140 ++++++++++++++++++ .../{ => calls}/ConstructorsTest.java | 14 +- .../{ => calls}/FunctionPointerTest.java | 2 +- .../{ => calls}/SuperCallTest.java | 17 ++- src/test/resources/calls/Calls.java | 37 +++++ src/test/resources/calls/External.java | 6 + src/test/resources/calls/SuperClass.java | 5 + src/test/resources/calls/calls.cpp | 61 ++++++++ 11 files changed, 281 insertions(+), 23 deletions(-) create mode 100644 src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/CallResolverTest.java rename src/test/java/de/fraunhofer/aisec/cpg/enhancements/{ => calls}/ConstructorsTest.java (91%) rename src/test/java/de/fraunhofer/aisec/cpg/enhancements/{ => calls}/FunctionPointerTest.java (99%) rename src/test/java/de/fraunhofer/aisec/cpg/enhancements/{ => calls}/SuperCallTest.java (90%) create mode 100644 src/test/resources/calls/Calls.java create mode 100644 src/test/resources/calls/External.java create mode 100644 src/test/resources/calls/SuperClass.java create mode 100644 src/test/resources/calls/calls.cpp diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java index 4a5ff14fd0..6ca1c319c4 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/Node.java @@ -238,6 +238,8 @@ public void setArgumentIndex(int argumentIndex) { this.argumentIndex = argumentIndex; } + /** @deprecated You should rather use {@link #isImplicit()} */ + @Deprecated(forRemoval = true) public boolean isDummy() { return dummy; } diff --git a/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java b/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java index 5021e0eaf0..a0f6b60132 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java @@ -49,20 +49,24 @@ public class TestUtils { - public static S findByPredicate(Collection nodes, Predicate predicate) { - List results = nodes.stream().filter(predicate).collect(Collectors.toList()); - assertEquals(1, results.size()); + public static S findByUniquePredicate( + Collection nodes, Predicate predicate) { + List results = findByPredicate(nodes, predicate); + assertEquals(1, results.size(), "Expected exactly one node matching the predicate"); return results.get(0); } + public static List findByPredicate( + Collection nodes, Predicate predicate) { + return nodes.stream().filter(predicate).collect(Collectors.toList()); + } + public static S findByUniqueName(Collection nodes, String name) { - List results = findByName(nodes, name); - assertEquals(1, results.size()); - return results.get(0); + return findByUniquePredicate(nodes, m -> m.getName().equals(name)); } public static List findByName(Collection nodes, String name) { - return nodes.stream().filter(m -> m.getName().equals(name)).collect(Collectors.toList()); + return findByPredicate(nodes, m -> m.getName().equals(name)); } /** diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/EOGTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/EOGTest.java index 9eddbe5ec0..f451e84aac 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/EOGTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/EOGTest.java @@ -271,7 +271,7 @@ void testCPPCallGraph() throws Exception { CallExpression third = TestUtils.findByUniqueName(calls, "third"); target = - TestUtils.findByPredicate( + TestUtils.findByUniquePredicate( functions, f -> f.getName().equals("third") && f.getParameters().size() == 2); assertEquals(List.of(target), third.getInvokes()); 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 new file mode 100644 index 0000000000..efabdd9aa9 --- /dev/null +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/CallResolverTest.java @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2020, Fraunhofer AISEC. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * $$$$$$\ $$$$$$$\ $$$$$$\ + * $$ __$$\ $$ __$$\ $$ __$$\ + * $$ / \__|$$ | $$ |$$ / \__| + * $$ | $$$$$$$ |$$ |$$$$\ + * $$ | $$ ____/ $$ |\_$$ | + * $$ | $$\ $$ | $$ | $$ | + * \$$$$$ |$$ | \$$$$$ | + * \______/ \__| \______/ + * + */ + +package de.fraunhofer.aisec.cpg.enhancements.calls; + +import static org.junit.jupiter.api.Assertions.*; + +import de.fraunhofer.aisec.cpg.BaseTest; +import de.fraunhofer.aisec.cpg.TestUtils; +import de.fraunhofer.aisec.cpg.graph.*; +import de.fraunhofer.aisec.cpg.graph.type.Type; +import de.fraunhofer.aisec.cpg.graph.type.TypeParser; +import java.nio.file.Path; +import java.util.List; +import org.junit.jupiter.api.Test; + +public class CallResolverTest extends BaseTest { + + private static final Path topLevel = Path.of("src", "test", "resources", "calls"); + + private void testMethods(List records, Type intType, Type stringType) { + RecordDeclaration callsRecord = TestUtils.findByUniqueName(records, "Calls"); + RecordDeclaration externalRecord = TestUtils.findByUniqueName(records, "External"); + RecordDeclaration superClassRecord = TestUtils.findByUniqueName(records, "SuperClass"); + + List innerMethods = + TestUtils.findByName( + TestUtils.subnodesOfType(callsRecord, MethodDeclaration.class), "innerTarget"); + List innerCalls = + TestUtils.findByName( + TestUtils.subnodesOfType(callsRecord, CallExpression.class), "innerTarget"); + checkCalls(intType, stringType, innerMethods, innerCalls); + + List superMethods = + TestUtils.findByName( + TestUtils.subnodesOfType(superClassRecord, MethodDeclaration.class), "superTarget"); + List superCalls = + TestUtils.findByName( + TestUtils.subnodesOfType(callsRecord, CallExpression.class), "superTarget"); + checkCalls(intType, stringType, superMethods, superCalls); + + List externalMethods = + TestUtils.findByName( + TestUtils.subnodesOfType(externalRecord, MethodDeclaration.class), "externalTarget"); + List externalCalls = + TestUtils.findByName( + TestUtils.subnodesOfType(callsRecord, CallExpression.class), "externalTarget"); + checkCalls(intType, stringType, externalMethods, externalCalls); + } + + private void ensureNoUnknownClassDummies(List records) { + RecordDeclaration callsRecord = TestUtils.findByUniqueName(records, "Calls"); + assertTrue(records.stream().noneMatch(r -> r.getName().equals("Unknown"))); + CallExpression unknownCall = + TestUtils.findByUniqueName( + TestUtils.subnodesOfType(callsRecord, CallExpression.class), "unknownTarget"); + assertEquals(List.of(), unknownCall.getInvokes()); + } + + private void checkCalls( + Type intType, + Type stringType, + List methods, + List calls) { + 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()); + } + + // 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()); + } + + @Test + void testJava() throws Exception { + List result = TestUtils.analyze("java", topLevel, true); + List records = TestUtils.subnodesOfType(result, RecordDeclaration.class); + Type intType = TypeParser.createFrom("int", true); + Type stringType = TypeParser.createFrom("java.lang.String", true); + + testMethods(records, intType, stringType); + ensureNoUnknownClassDummies(records); + } + + @Test + void testCpp() throws Exception { + List result = TestUtils.analyze("cpp", topLevel, true); + List records = TestUtils.subnodesOfType(result, RecordDeclaration.class); + Type intType = TypeParser.createFrom("int", true); + Type stringType = TypeParser.createFrom("char*", true); + + testMethods(records, intType, stringType); + + // Test functions (not methods!) + List functions = + TestUtils.findByPredicate( + TestUtils.subnodesOfType(result, FunctionDeclaration.class), + f -> !(f instanceof MethodDeclaration)); + List calls = + TestUtils.findByName( + TestUtils.subnodesOfType(result, CallExpression.class), "functionTarget"); + checkCalls(intType, stringType, functions, calls); + + ensureNoUnknownClassDummies(records); + } +} diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/ConstructorsTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/ConstructorsTest.java similarity index 91% rename from src/test/java/de/fraunhofer/aisec/cpg/enhancements/ConstructorsTest.java rename to src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/ConstructorsTest.java index cd56c47cd8..32a02cf4a3 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/ConstructorsTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/ConstructorsTest.java @@ -1,4 +1,4 @@ -package de.fraunhofer.aisec.cpg.enhancements; +package de.fraunhofer.aisec.cpg.enhancements.calls; import static org.junit.jupiter.api.Assertions.*; @@ -23,11 +23,11 @@ void testJava() throws Exception { List constructors = TestUtils.subnodesOfType(result, ConstructorDeclaration.class); ConstructorDeclaration noArg = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 0); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 0); ConstructorDeclaration singleArg = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 1); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 1); ConstructorDeclaration twoArgs = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 2); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 2); List variables = TestUtils.subnodesOfType(result, VariableDeclaration.class); @@ -66,11 +66,11 @@ void testCPP() throws Exception { List constructors = TestUtils.subnodesOfType(result, ConstructorDeclaration.class); ConstructorDeclaration noArg = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 0); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 0); ConstructorDeclaration singleArg = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 1); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 1); ConstructorDeclaration twoArgs = - TestUtils.findByPredicate(constructors, c -> c.getParameters().size() == 2); + TestUtils.findByUniquePredicate(constructors, c -> c.getParameters().size() == 2); List variables = TestUtils.subnodesOfType(result, VariableDeclaration.class); diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/FunctionPointerTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java similarity index 99% rename from src/test/java/de/fraunhofer/aisec/cpg/enhancements/FunctionPointerTest.java rename to src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java index 9ddb37374f..b24775666a 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/FunctionPointerTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java @@ -24,7 +24,7 @@ * */ -package de.fraunhofer.aisec.cpg.enhancements; +package de.fraunhofer.aisec.cpg.enhancements.calls; import static org.junit.jupiter.api.Assertions.*; diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/SuperCallTest.java similarity index 90% rename from src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java rename to src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/SuperCallTest.java index 38ab9e3a10..d1c690e9c2 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/SuperCallTest.java @@ -1,4 +1,4 @@ -package de.fraunhofer.aisec.cpg.enhancements; +package de.fraunhofer.aisec.cpg.enhancements.calls; import static org.junit.jupiter.api.Assertions.*; @@ -30,7 +30,7 @@ void testSimpleCall() throws Exception { MethodDeclaration target = TestUtils.findByUniqueName(methods, "target"); List calls = TestUtils.subnodesOfType(target, CallExpression.class); CallExpression superCall = - TestUtils.findByPredicate(calls, c -> "super.target();".equals(c.getCode())); + TestUtils.findByUniquePredicate(calls, c -> "super.target();".equals(c.getCode())); assertEquals(List.of(superTarget), superCall.getInvokes()); } @@ -55,9 +55,11 @@ void testInterfaceCall() throws Exception { MethodDeclaration target = TestUtils.findByUniqueName(methods, "target"); List calls = TestUtils.subnodesOfType(target, CallExpression.class); CallExpression interface1Call = - TestUtils.findByPredicate(calls, c -> "Interface1.super.target();".equals(c.getCode())); + TestUtils.findByUniquePredicate( + calls, c -> "Interface1.super.target();".equals(c.getCode())); CallExpression interface2Call = - TestUtils.findByPredicate(calls, c -> "Interface2.super.target();".equals(c.getCode())); + TestUtils.findByUniquePredicate( + calls, c -> "Interface2.super.target();".equals(c.getCode())); assertEquals(List.of(interface1Target), interface1Call.getInvokes()); assertEquals(List.of(interface2Target), interface2Call.getInvokes()); @@ -77,12 +79,13 @@ void testSuperField() throws Exception { MethodDeclaration getField = TestUtils.findByUniqueName(methods, "getField"); List refs = TestUtils.subnodesOfType(getField, MemberExpression.class); - MemberExpression fieldRef = TestUtils.findByPredicate(refs, r -> "field".equals(r.getCode())); + MemberExpression fieldRef = + TestUtils.findByUniquePredicate(refs, r -> "field".equals(r.getCode())); MethodDeclaration getSuperField = TestUtils.findByUniqueName(methods, "getSuperField"); refs = TestUtils.subnodesOfType(getSuperField, MemberExpression.class); MemberExpression superFieldRef = - TestUtils.findByPredicate(refs, r -> "super.field".equals(r.getCode())); + TestUtils.findByUniquePredicate(refs, r -> "super.field".equals(r.getCode())); assertEquals(subClass.getThis(), fieldRef.getBase()); assertEquals(field, fieldRef.getMember()); @@ -105,7 +108,7 @@ void testInnerCall() throws Exception { MethodDeclaration target = TestUtils.findByUniqueName(methods, "inner"); List calls = TestUtils.subnodesOfType(target, CallExpression.class); CallExpression superCall = - TestUtils.findByPredicate(calls, c -> "SubClass.super.target();".equals(c.getCode())); + TestUtils.findByUniquePredicate(calls, c -> "SubClass.super.target();".equals(c.getCode())); assertEquals(List.of(superTarget), superCall.getInvokes()); } diff --git a/src/test/resources/calls/Calls.java b/src/test/resources/calls/Calls.java new file mode 100644 index 0000000000..5924540c6f --- /dev/null +++ b/src/test/resources/calls/Calls.java @@ -0,0 +1,37 @@ +public class Calls extends SuperClass { + + private void innerTarget() {} + private void innerTarget(int param1, int param2) {} + private void innerTarget(int param1, String param2) {} + + public static void main(String[] args) { + innerTarget(); + innerTarget(1, 2); + innerTarget(1, "2"); + // dummy + innerTarget(1, 2, 3); + + superTarget(); + superTarget(1, 2); + superTarget(1, "2"); + // dummy + superTarget(1, 2, 3); + + External e = new External(); + e.externalTarget(); + e.externalTarget(1, 2); + e.externalTarget(1, "2"); + // dummy + e.externalTarget(1, 2, 3); + + 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! + u.unknownTarget(); + } +} \ No newline at end of file diff --git a/src/test/resources/calls/External.java b/src/test/resources/calls/External.java new file mode 100644 index 0000000000..e488114464 --- /dev/null +++ b/src/test/resources/calls/External.java @@ -0,0 +1,6 @@ +public class External extends SuperClass { + + public void externalTarget() {} + public void externalTarget(int param1, int param2) {} + public void externalTarget(int param1, int param2) {} +} \ No newline at end of file diff --git a/src/test/resources/calls/SuperClass.java b/src/test/resources/calls/SuperClass.java new file mode 100644 index 0000000000..d7c1397b6a --- /dev/null +++ b/src/test/resources/calls/SuperClass.java @@ -0,0 +1,5 @@ +public class SuperClass { + public void superTarget() {} + public void superTarget(int param1, int param2) {} + public void superTarget(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 new file mode 100644 index 0000000000..05638ec3fe --- /dev/null +++ b/src/test/resources/calls/calls.cpp @@ -0,0 +1,61 @@ +void functionTarget() {} +void functionTarget(int param1, int param2) {} +void functionTarget(int param1, const char* param2) {} + +class SuperClass { + public: + void superTarget() {} + void superTarget(int param1, int param2) {} + void superTarget(int param1, const char* param2) {} +}; + +class External: public SuperClass { + public: + void externalTarget() {} + void externalTarget(int param1, int param2) {} + void externalTarget(int param1, const char* param2) {} +}; + +class Calls: SuperClass { + private: + void innerTarget() {} + void innerTarget(int param1, int param2) {} + void innerTarget(int param1, const char* param2) {} + public: + void main() { + functionTarget(); + functionTarget(1, 2); + functionTarget(1, "2"); + // dummy + functionTarget(1, 2, 3); + + innerTarget(); + innerTarget(1, 2); + innerTarget(1, "2"); + // dummy + innerTarget(1, 2, 3); + + superTarget(); + superTarget(1, 2); + superTarget(1, "2"); + // dummy + superTarget(1, 2, 3); + + External e; + e.externalTarget(); + e.externalTarget(1, 2); + e.externalTarget(1, "2"); + // dummy + e.externalTarget(1, 2, 3); + + 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 From 61f4253dd252591717ab79f52a5b625fd80d3289 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Fri, 14 Aug 2020 16:19:02 +0200 Subject: [PATCH 3/5] #135 create dummies for explicit constructor invocations, revamp fptr dummy behavior --- .../frontends/java/JavaLanguageFrontend.java | 7 +- .../cpg/frontends/java/StatementAnalyzer.java | 12 +- .../cpg/graph/ConstructorDeclaration.java | 15 ++- .../aisec/cpg/passes/CallResolver.java | 126 ++++++------------ .../cpg/passes/VariableUsageResolver.java | 34 +++-- .../calls/FunctionPointerTest.java | 38 ++---- 6 files changed, 89 insertions(+), 143 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java index 20b5d43e12..b517bde4dd 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java @@ -60,7 +60,9 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -291,8 +293,7 @@ public String recoverTypeFromUnsolvedException(Throwable ex) { qualifier = ((UnsolvedSymbolException) ex.getCause()).getName(); } // this comes from the Javaparser! - if (qualifier.startsWith("We are unable to find the value declaration corresponding to") - || qualifier.startsWith("Solving ")) { + if (qualifier.startsWith("We are unable to find") || qualifier.startsWith("Solving ")) { return null; } String fromImport = getQualifiedNameFromImports(qualifier); diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java index a9f596a904..df229735df 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java @@ -557,12 +557,12 @@ public SwitchStatement handleSwitchStatement(Statement stmt) { private ExplicitConstructorInvocation handleExplicitConstructorInvocation(Statement stmt) { ExplicitConstructorInvocationStmt eciStatement = stmt.asExplicitConstructorInvocationStmt(); - String containingClass; - try { - containingClass = eciStatement.resolve().declaringType().getQualifiedName(); - } catch (RuntimeException | NoClassDefFoundError e) { - containingClass = lang.recoverTypeFromUnsolvedException(e); - // base can be null here + String containingClass = ""; + RecordDeclaration currentRecord = lang.getScopeManager().getCurrentRecord(); + if (currentRecord == null) { + log.error("Explicit constructor invocation has to be located inside a record declaration!"); + } else { + containingClass = currentRecord.getName(); } ExplicitConstructorInvocation node = diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java index 7a6efb08bd..f99e66234c 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java @@ -27,6 +27,7 @@ package de.fraunhofer.aisec.cpg.graph; import de.fraunhofer.aisec.cpg.graph.type.TypeParser; +import org.checkerframework.checker.nullness.qual.Nullable; /** * The declaration of a constructor within a {@link RecordDeclaration}. Is it essentially a special @@ -47,11 +48,6 @@ public static ConstructorDeclaration from(MethodDeclaration methodDeclaration) { NodeBuilder.newConstructorDeclaration( methodDeclaration.getName(), methodDeclaration.getCode(), recordDeclaration); - if (recordDeclaration != null) { - // constructors always have implicitly the return type of their class - c.setType(TypeParser.createFrom(recordDeclaration.getName(), true)); - } - c.setBody(methodDeclaration.getBody()); c.setLocation(methodDeclaration.getLocation()); c.setParameters(methodDeclaration.getParameters()); @@ -66,4 +62,13 @@ public static ConstructorDeclaration from(MethodDeclaration methodDeclaration) { return c; } + + @Override + public void setRecordDeclaration(@Nullable RecordDeclaration recordDeclaration) { + super.setRecordDeclaration(recordDeclaration); + if (recordDeclaration != null) { + // constructors always have implicitly the return type of their class + setType(TypeParser.createFrom(recordDeclaration.getName(), true)); + } + } } 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 ac0d7a604e..e0d22db260 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java @@ -332,56 +332,40 @@ private void resolveConstructExpression(ConstructExpression constructExpression) if (record != null && record.getCode() != null && !record.getCode().isEmpty()) { ConstructorDeclaration constructor = getConstructorDeclaration(signature, record); - if (constructor != null) { - constructExpression.setConstructor(constructor); - } else { - LOGGER.warn( - "Unexpected: Could not find constructor for {} with signature {}", - record.getName(), - signature); - } + constructExpression.setConstructor(constructor); } } private void handleFunctionPointerCall(CallExpression call, Node pointer) { + if (!(pointer instanceof HasType + && ((HasType) pointer).getType() instanceof FunctionPointerType)) { + LOGGER.error("Can't handle a function pointer call without function pointer type"); + return; + } + FunctionPointerType pointerType = (FunctionPointerType) ((HasType) pointer).getType(); List invocationCandidates = new ArrayList<>(); Deque worklist = new ArrayDeque<>(); Set seen = Collections.newSetFromMap(new IdentityHashMap<>()); worklist.push(pointer); - DeclaredReferenceExpression finalReference = null; while (!worklist.isEmpty()) { Node curr = worklist.pop(); if (!seen.add(curr)) { continue; } if (curr instanceof FunctionDeclaration) { - if (((FunctionDeclaration) curr).hasSignature(call.getSignature())) { + FunctionDeclaration f = (FunctionDeclaration) curr; + // Even if it is a function declaration, the dataflow might just come from a situation + // where the target of a fptr is passed through via a return value. Keep searching if + // return type or signature don't match + if (TypeManager.getInstance().isSupertypeOf(pointerType.getReturnType(), f.getType()) + && f.hasSignature(pointerType.getParameters())) { invocationCandidates.add((FunctionDeclaration) curr); - } else if (curr.isImplicit()) { - // unknown function, so even if its signature does not fit, we need to make a new - // dummy that does match - if (((FunctionDeclaration) curr).hasSignature(call.getSignature())) { - invocationCandidates.add((FunctionDeclaration) curr); - // refine the referral pointer - if (finalReference != null && finalReference.getRefersTo().contains(curr)) { - finalReference.setRefersTo((ValueDeclaration) curr); - } - } else { - FunctionDeclaration dummy = - createDummyWithMatchingSignature((FunctionDeclaration) curr, call.getSignature()); - invocationCandidates.add(dummy); - // redirect the referral pointer - if (finalReference != null && finalReference.getRefersTo().contains(curr)) { - finalReference.setRefersTo(dummy); - } - } - } - } else { - if (curr instanceof DeclaredReferenceExpression) { - finalReference = (DeclaredReferenceExpression) curr; + // We have found a target. Don't follow this path any further, but still continue the + // other paths that might be left, as we could have several potential targets at runtime + continue; } - curr.getPrevDFG().forEach(worklist::push); } + curr.getPrevDFG().forEach(worklist::push); } call.setInvokes(invocationCandidates); } @@ -394,9 +378,7 @@ private void resolveExplicitConstructorInvocation(ExplicitConstructorInvocation if (record != null) { ConstructorDeclaration constructor = getConstructorDeclaration(signature, record); ArrayList invokes = new ArrayList<>(); - if (constructor != null) { - invokes.add(constructor); - } + invokes.add(constructor); eci.setInvokes(invokes); } } @@ -463,65 +445,25 @@ private void generateStaticImportDummies( } } - private Optional checkExistingDummies( - FunctionDeclaration template, List signature) { - if (template instanceof MethodDeclaration - && ((MethodDeclaration) template).getRecordDeclaration() != null) { - return ((MethodDeclaration) template) - .getRecordDeclaration().getMethods().stream() - .filter(m -> m.getName().equals(template.getName()) && m.hasSignature(signature)) - .map(FunctionDeclaration.class::cast) - .findFirst(); - } else { - if (currentTU == null) { - LOGGER.error( - "No current translation unit when trying to find matching dummy for {}", template); - return Optional.empty(); - } - return currentTU.getDeclarations().stream() - .filter(FunctionDeclaration.class::isInstance) - .map(FunctionDeclaration.class::cast) - .filter(f -> f.getName().equals(template.getName()) && f.hasSignature(signature)) - .findFirst(); - } - } - private FunctionDeclaration createDummyWithMatchingSignature( - FunctionDeclaration template, List signature) { - Optional existing = checkExistingDummies(template, signature); - if (existing.isPresent()) { - return existing.get(); - } + RecordDeclaration containingRecord, + String name, + String code, + boolean isStatic, + List signature) { List parameters = Util.createParameters(signature); - if (template instanceof MethodDeclaration) { - RecordDeclaration containingRecord = ((MethodDeclaration) template).getRecordDeclaration(); + if (containingRecord != null) { MethodDeclaration dummy = - NodeBuilder.newMethodDeclaration( - template.getName(), - template.getCode(), - ((MethodDeclaration) template).isStatic(), - containingRecord); + NodeBuilder.newMethodDeclaration(name, code, isStatic, containingRecord); dummy.setImplicit(true); dummy.setParameters(parameters); - if (containingRecord == null) { - // not inside a class, lets put it inside the translation unit - if (currentTU == null) { - LOGGER.error( - "No current translation unit when trying to generate method dummy {}", - dummy.getName()); - } else { - currentTU.getDeclarations().add(dummy); - } - } else { - containingRecord.getMethods().add(dummy); - } + containingRecord.getMethods().add(dummy); return dummy; } else { // function declaration, not inside a class - FunctionDeclaration dummy = - NodeBuilder.newFunctionDeclaration(template.getName(), template.getCode()); + FunctionDeclaration dummy = NodeBuilder.newFunctionDeclaration(name, code); dummy.setParameters(parameters); dummy.setImplicit(true); if (currentTU == null) { @@ -535,6 +477,16 @@ private FunctionDeclaration createDummyWithMatchingSignature( } } + private ConstructorDeclaration createConstructorDummy( + @NonNull RecordDeclaration containingRecord, List signature) { + ConstructorDeclaration dummy = + NodeBuilder.newConstructorDeclaration(containingRecord.getName(), "", containingRecord); + dummy.setImplicit(true); + dummy.setParameters(Util.createParameters(signature)); + containingRecord.getConstructors().add(dummy); + return dummy; + } + private Set getPossibleContainingTypes(Node node, RecordDeclaration curClass) { Set possibleTypes = new HashSet<>(); if (node instanceof MemberCallExpression) { @@ -595,12 +547,12 @@ private Set getOverridingCandidates( .collect(Collectors.toSet()); } - @Nullable + @NonNull private ConstructorDeclaration getConstructorDeclaration( List signature, RecordDeclaration record) { return record.getConstructors().stream() .filter(f -> f.hasSignature(signature)) .findFirst() - .orElse(null); + .orElseGet(() -> createConstructorDummy(record, signature)); } } diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java index 49e54a32e4..ca0af6ec87 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -175,10 +175,12 @@ private Set resolveFunctionPtr( } if (containingClass == null) { - targets.add(handleUnknownMethod(functionName, reference.getType())); + targets.add( + handleUnknownMethod(functionName, fptrType.getReturnType(), fptrType.getParameters())); } else { MethodDeclaration resolved = - handleUnknownClassMethod(containingClass, functionName, reference.getType()); + handleUnknownClassMethod( + containingClass, functionName, fptrType.getReturnType(), fptrType.getParameters()); if (resolved != null) { targets.add(resolved); } @@ -385,18 +387,24 @@ private FieldDeclaration handleUnknownField(Type base, String name, Type type) { } } - private MethodDeclaration handleUnknownClassMethod(Type base, String name, Type type) { + private MethodDeclaration handleUnknownClassMethod( + Type base, String name, Type returnType, List signature) { if (!recordMap.containsKey(base)) { return null; } RecordDeclaration containingRecord = recordMap.get(base); List declarations = containingRecord.getMethods(); Optional target = - declarations.stream().filter(f -> f.getName().equals(name)).findFirst(); + declarations.stream() + .filter(f -> f.getName().equals(name)) + .filter(f -> f.getType().equals(returnType)) + .filter(f -> f.hasSignature(signature)) + .findFirst(); if (target.isEmpty()) { MethodDeclaration declaration = NodeBuilder.newMethodDeclaration(name, "", false, containingRecord); - declaration.setType(type); + declaration.setType(returnType); + declaration.setParameters(Util.createParameters(signature)); declarations.add(declaration); declaration.setImplicit(true); return declaration; @@ -405,23 +413,21 @@ private MethodDeclaration handleUnknownClassMethod(Type base, String name, Type } } - private FunctionDeclaration handleUnknownMethod(String name, Type type) { + private FunctionDeclaration handleUnknownMethod( + String name, Type returnType, List signature) { Optional target = currTu.getDeclarations().stream() .filter(FunctionDeclaration.class::isInstance) .map(FunctionDeclaration.class::cast) .filter(f -> f.getName().equals(name)) - .filter(f -> f.hasSignature(((FunctionPointerType) type).getParameters())) + .filter(f -> f.getType().equals(returnType)) + .filter(f -> f.hasSignature(signature)) .findFirst(); if (target.isEmpty()) { FunctionDeclaration declaration = NodeBuilder.newFunctionDeclaration(name, ""); - if (type instanceof FunctionPointerType) { - declaration.setType(((FunctionPointerType) type).getReturnType()); - declaration.setParameters( - Util.createParameters(((FunctionPointerType) type).getParameters())); - } else { - declaration.setType(type); - } + declaration.setType(returnType); + declaration.setParameters(Util.createParameters(signature)); + currTu.add(declaration); declaration.setImplicit(true); return declaration; diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java index b24775666a..376e347320 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java @@ -32,21 +32,11 @@ import de.fraunhofer.aisec.cpg.TestUtils; import de.fraunhofer.aisec.cpg.TranslationConfiguration; import de.fraunhofer.aisec.cpg.TranslationManager; -import de.fraunhofer.aisec.cpg.graph.CallExpression; -import de.fraunhofer.aisec.cpg.graph.FunctionDeclaration; -import de.fraunhofer.aisec.cpg.graph.Node; -import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; -import de.fraunhofer.aisec.cpg.graph.VariableDeclaration; +import de.fraunhofer.aisec.cpg.graph.*; import java.io.File; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Deque; -import java.util.IdentityHashMap; -import java.util.List; -import java.util.Set; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -85,25 +75,17 @@ void test(String language) throws Exception { FunctionDeclaration main = TestUtils.findByUniqueName(functions, "main"); List calls = TestUtils.subnodesOfType(main, CallExpression.class); FunctionDeclaration noParam = - functions.stream() - .filter(f -> f.getName().equals("target") && f.getParameters().isEmpty()) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("target") && f.getParameters().isEmpty()); FunctionDeclaration singleParam = - functions.stream() - .filter(f -> f.getName().equals("target") && f.getParameters().size() == 1) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("target") && f.getParameters().size() == 1); FunctionDeclaration noParamUnknown = - functions.stream() - .filter(f -> f.getName().equals("fun") && f.getParameters().isEmpty()) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("fun") && f.getParameters().isEmpty()); FunctionDeclaration singleParamUnknown = - functions.stream() - .filter(f -> f.getName().equals("fun") && f.getParameters().size() == 1) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("fun") && f.getParameters().size() == 1); Pattern pattern = Pattern.compile("\\((?.+)?\\*(?.+\\.)?(?.+)\\)"); for (CallExpression call : calls) { String func; From 22bd52c44a303a8edf119b11f1aa1c747683e4f5 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Mon, 17 Aug 2020 16:08:19 +0200 Subject: [PATCH 4/5] #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 From 9a368d2b122322a94d9b435925fc44164693d28b Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 20 Aug 2020 13:42:38 +0200 Subject: [PATCH 5/5] #135 Add test for calls to overridden methods --- .../aisec/cpg/passes/scopes/ScopeManager.java | 9 +++---- .../aisec/cpg/enhancements/JavaVsCppTest.java | 2 +- .../enhancements/calls/CallResolverTest.java | 25 +++++++++++++++++++ src/test/resources/calls/Calls.java | 3 +++ src/test/resources/calls/External.java | 3 +++ src/test/resources/calls/SuperClass.java | 2 ++ src/test/resources/calls/calls.cpp | 5 ++++ 7 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/scopes/ScopeManager.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/scopes/ScopeManager.java index 83b60d0a02..5d4caf5dd3 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/scopes/ScopeManager.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/scopes/ScopeManager.java @@ -440,13 +440,12 @@ public void resetToGlobal() { } } - /** - * Adds a declaration to the CPG by taking into account the currently active scope, and add the Declaration to the - * appropriate node. This function will keep the declaration in the Scopes and allows the ScopeManager by himself to - * resolve ValueDeclarations through. + * Adds a declaration to the CPG by taking into account the currently active scope, and add the + * Declaration to the appropriate node. This function will keep the declaration in the Scopes and + * allows the ScopeManager by himself to resolve ValueDeclarations through. * - * {@link ScopeManager#resolve(DeclaredReferenceExpression)}. + *

{@link ScopeManager#resolve(DeclaredReferenceExpression)}. * * @param declaration */ 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 e2c77911d9..b09351b0a2 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(2, rec.getMethods().size()); // printf dummy + 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/calls/CallResolverTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/CallResolverTest.java index 35e32f0af3..092234d0c3 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 @@ -113,6 +113,29 @@ private void checkCalls( } } + private void testOverriding(List records) { + RecordDeclaration callsRecord = TestUtils.findByUniqueName(records, "Calls"); + RecordDeclaration externalRecord = TestUtils.findByUniqueName(records, "External"); + RecordDeclaration superClassRecord = TestUtils.findByUniqueName(records, "SuperClass"); + + MethodDeclaration originalMethod = + TestUtils.findByUniqueName( + TestUtils.subnodesOfType(superClassRecord, MethodDeclaration.class), + "overridingTarget"); + MethodDeclaration overridingMethod = + TestUtils.findByUniqueName( + TestUtils.subnodesOfType(externalRecord, MethodDeclaration.class), "overridingTarget"); + CallExpression call = + TestUtils.findByUniqueName( + TestUtils.subnodesOfType(callsRecord, CallExpression.class), "overridingTarget"); + + // TODO related to #204: Currently we have both the original and the overriding method in the + // invokes list. This check needs to be adjusted to the choice we make on solving #204 + assertTrue(call.getInvokes().contains(overridingMethod)); + assertEquals(List.of(originalMethod), overridingMethod.getOverrides()); + assertEquals(List.of(overridingMethod), originalMethod.getOverriddenBy()); + } + @Test void testJava() throws Exception { List result = TestUtils.analyze("java", topLevel, true); @@ -121,6 +144,7 @@ void testJava() throws Exception { Type stringType = TypeParser.createFrom("java.lang.String", true); testMethods(records, intType, stringType); + testOverriding(records); ensureNoUnknownClassDummies(records); } @@ -132,6 +156,7 @@ void testCpp() throws Exception { Type stringType = TypeParser.createFrom("char*", true); testMethods(records, intType, stringType); + testOverriding(records); // Test functions (not methods!) List functions = diff --git a/src/test/resources/calls/Calls.java b/src/test/resources/calls/Calls.java index eb8e63a6f4..7339545b70 100644 --- a/src/test/resources/calls/Calls.java +++ b/src/test/resources/calls/Calls.java @@ -28,6 +28,9 @@ public static void main(String[] args) { e.superTarget(1, 2); e.superTarget(1, "2"); + SuperClass s = new External(); + s.overridingTarget(); + Unknown u = new Unknown(); // don't create dummy for methods of unknown classes! u.unknownTarget(); diff --git a/src/test/resources/calls/External.java b/src/test/resources/calls/External.java index 59381c901f..5be08328b7 100644 --- a/src/test/resources/calls/External.java +++ b/src/test/resources/calls/External.java @@ -3,4 +3,7 @@ public class External extends SuperClass { public void externalTarget() {} public void externalTarget(int param1, int param2) {} public void externalTarget(int param1, String param2) {} + + @Override + public void overridingTarget() {} } \ No newline at end of file diff --git a/src/test/resources/calls/SuperClass.java b/src/test/resources/calls/SuperClass.java index d7c1397b6a..4ac539fd54 100644 --- a/src/test/resources/calls/SuperClass.java +++ b/src/test/resources/calls/SuperClass.java @@ -2,4 +2,6 @@ public class SuperClass { public void superTarget() {} public void superTarget(int param1, int param2) {} public void superTarget(int param1, String param2) {} + + public void overridingTarget() {} } \ No newline at end of file diff --git a/src/test/resources/calls/calls.cpp b/src/test/resources/calls/calls.cpp index 47dd8a6690..962f579cac 100644 --- a/src/test/resources/calls/calls.cpp +++ b/src/test/resources/calls/calls.cpp @@ -7,6 +7,7 @@ class SuperClass { void superTarget() {} void superTarget(int param1, int param2) {} void superTarget(int param1, const char* param2) {} + virtual void overridingTarget() {} }; class External: public SuperClass { @@ -14,6 +15,7 @@ class External: public SuperClass { void externalTarget() {} void externalTarget(int param1, int param2) {} void externalTarget(int param1, const char* param2) {} + void overridingTarget() override {} }; class Calls: SuperClass { @@ -50,6 +52,9 @@ class Calls: SuperClass { e.superTarget(1, 2); e.superTarget(1, "2"); + SuperClass *s = new External(); + s->overridingTarget(); + Unknown u; // don't create dummy for methods of unknown classes! u.unknownTarget();