From f27c53d4d7caead5cdefdc060fb237f536580182 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Fri, 22 May 2020 13:59:07 +0200 Subject: [PATCH 1/9] #99 correctly resolve super field access --- .../cpg/frontends/java/ExpressionHandler.java | 16 ++++ .../aisec/cpg/graph/RecordDeclaration.java | 6 ++ .../cpg/passes/VariableUsageResolver.java | 79 ++++++++++++++----- 3 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/ExpressionHandler.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/ExpressionHandler.java index dc3e2f14ca8..4ff8c20ee70 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/ExpressionHandler.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/ExpressionHandler.java @@ -62,6 +62,7 @@ public ExpressionHandler(JavaLanguageFrontend lang) { map.put(LiteralExpr.class, this::handleLiteralExpression); map.put(ThisExpr.class, this::handleThisExpression); + map.put(SuperExpr.class, this::handleSuperExpression); map.put(ClassExpr.class, this::handleClassExpression); map.put(NameExpr.class, this::handleNameExpression); map.put(InstanceOfExpr.class, this::handleInstanceOfExpression); @@ -432,6 +433,18 @@ private DeclaredReferenceExpression handleThisExpression(Expression expr) { return thisExpression; } + private DeclaredReferenceExpression handleSuperExpression(Expression expr) { + // The actual type is hard to determine at this point, as we may not have full information + // about the inheritance structure. Thus we delay the resolving to the variable resolving + // process + DeclaredReferenceExpression superExpression = + NodeBuilder.newDeclaredReferenceExpression( + expr.toString(), UnknownType.getUnknownType(), expr.toString()); + lang.setCodeAndRegion(superExpression, expr); + + return superExpression; + } + // TODO: this function needs a MAJOR overhaul! private de.fraunhofer.aisec.cpg.graph.Expression handleNameExpression(Expression expr) { NameExpr nameExpr = expr.asNameExpr(); @@ -618,6 +631,9 @@ private CallExpression handleMethodCallExpression(Expression expr) { scopeName = ((NameExpr) scope).getNameAsString(); ((NameExpr) scope).resolve(); isresolvable = true; + } else if (scope instanceof SuperExpr) { + scopeName = scope.toString(); + isresolvable = true; } } catch (UnsolvedSymbolException ex) { if (!ex.getName() diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java index 9ef005e8aa4..a6a2d6fa05e 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java @@ -91,6 +91,12 @@ public FieldDeclaration getThis() { return fields.stream().filter(f -> f.getName().equals("this")).findFirst().orElse(null); } + public FieldDeclaration getSuper() { + Optional superType = + getSuperTypeDeclarations().stream().filter(r -> r.getKind().equals("class")).findFirst(); + return superType.map(RecordDeclaration::getThis).orElse(null); + } + public List getMethods() { return methods; } 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 6a6c57cc07f..5aad52ab6f3 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -29,14 +29,37 @@ import static de.fraunhofer.aisec.cpg.helpers.Util.warnWithFileLocation; import de.fraunhofer.aisec.cpg.TranslationResult; -import de.fraunhofer.aisec.cpg.graph.*; +import de.fraunhofer.aisec.cpg.frontends.java.JavaLanguageFrontend; +import de.fraunhofer.aisec.cpg.graph.Declaration; +import de.fraunhofer.aisec.cpg.graph.DeclaredReferenceExpression; +import de.fraunhofer.aisec.cpg.graph.EnumDeclaration; +import de.fraunhofer.aisec.cpg.graph.FieldDeclaration; +import de.fraunhofer.aisec.cpg.graph.FunctionDeclaration; +import de.fraunhofer.aisec.cpg.graph.HasType; +import de.fraunhofer.aisec.cpg.graph.MemberCallExpression; +import de.fraunhofer.aisec.cpg.graph.MemberExpression; +import de.fraunhofer.aisec.cpg.graph.MethodDeclaration; +import de.fraunhofer.aisec.cpg.graph.Node; +import de.fraunhofer.aisec.cpg.graph.NodeBuilder; +import de.fraunhofer.aisec.cpg.graph.RecordDeclaration; +import de.fraunhofer.aisec.cpg.graph.StaticReferenceExpression; +import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; +import de.fraunhofer.aisec.cpg.graph.ValueDeclaration; import de.fraunhofer.aisec.cpg.graph.type.FunctionPointerType; import de.fraunhofer.aisec.cpg.graph.type.Type; import de.fraunhofer.aisec.cpg.graph.type.TypeParser; import de.fraunhofer.aisec.cpg.graph.type.UnknownType; import de.fraunhofer.aisec.cpg.helpers.SubgraphWalker.ScopedWalker; import de.fraunhofer.aisec.cpg.helpers.Util; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -184,9 +207,18 @@ private Set resolveFunctionPtr( private void resolveLocalVarUsage(RecordDeclaration currentClass, Node parent, Node current) { if (current instanceof DeclaredReferenceExpression) { DeclaredReferenceExpression ref = (DeclaredReferenceExpression) current; + if (parent instanceof MemberCallExpression + && current == ((MemberCallExpression) parent).getMember() + && !(ref.getType() instanceof FunctionPointerType)) { + // members of a MemberCallExpression are no variables to be resolved, unless we have a + // function pointer call + return; + } Set refersTo = walker - .getDeclarationForScope(parent, v -> v.getName().equals(ref.getName())) + .getDeclarationForScope( + parent, + v -> !(v instanceof FunctionDeclaration) && v.getName().equals(ref.getName())) .map( d -> { Set set = new HashSet<>(); @@ -200,12 +232,7 @@ private void resolveLocalVarUsage(RecordDeclaration currentClass, Node parent, N recordDeclType = TypeParser.createFrom(currentClass.getName(), true); } - if (ref.getType() instanceof FunctionPointerType - && (refersTo.isEmpty() - || refersTo.stream().anyMatch(FunctionDeclaration.class::isInstance))) { - // If we already found something, this might either be a function pointer variable or a - // function that would match the name. If we found a function, discard this finding, as - // it is most likely not correct yet + if (ref.getType() instanceof FunctionPointerType && refersTo.isEmpty()) { refersTo = resolveFunctionPtr(recordDeclType, ref); } @@ -215,9 +242,13 @@ private void resolveLocalVarUsage(RecordDeclaration currentClass, Node parent, N && recordDeclType != null && recordMap.containsKey(recordDeclType)) { // Maybe we are referring to a field instead of a local var - Set resolvedMember = new HashSet<>(); - resolvedMember.add(resolveMember(recordDeclType, (DeclaredReferenceExpression) current)); - refersTo = resolvedMember; + ValueDeclaration field = + resolveMember(recordDeclType, (DeclaredReferenceExpression) current); + if (field != null) { + Set resolvedMember = new HashSet<>(); + resolvedMember.add(field); + refersTo = resolvedMember; + } } if (!refersTo.isEmpty()) { @@ -234,7 +265,11 @@ private void resolveFieldUsages(Node current, RecordDeclaration curClass) { Node base = memberExpression.getBase(); Node member = memberExpression.getMember(); if (base instanceof DeclaredReferenceExpression) { - base = resolveBase((DeclaredReferenceExpression) memberExpression.getBase()); + if (lang instanceof JavaLanguageFrontend && base.getName().equals("super")) { + base = curClass.getSuper(); + } else { + base = resolveBase((DeclaredReferenceExpression) memberExpression.getBase()); + } } if (member instanceof DeclaredReferenceExpression) { if (base instanceof EnumDeclaration) { @@ -312,16 +347,20 @@ private Declaration resolveBase(DeclaredReferenceExpression reference) { private ValueDeclaration resolveMember( Type containingClass, DeclaredReferenceExpression reference) { + if (lang instanceof JavaLanguageFrontend && reference.getName().equals("super")) { + // if we have a "super" on the member side, this is a member call. We need to resolve this + // in the call resolver instead + return null; + } Optional member = Optional.empty(); - if (!TypeManager.getInstance().isUnknown(containingClass)) { - if (recordMap.containsKey(containingClass)) { - member = - recordMap.get(containingClass).getFields().stream() - .filter(f -> f.getName().equals(reference.getName())) - .findFirst(); - } + if (!(containingClass instanceof UnknownType) && recordMap.containsKey(containingClass)) { + member = + recordMap.get(containingClass).getFields().stream() + .filter(f -> f.getName().equals(reference.getName())) + .findFirst(); } + if (member.isEmpty()) { member = superTypesMap.getOrDefault(containingClass, Collections.emptyList()).stream() From eee47a0b1e219ddc294a8139f28cecd3e5b166f8 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 4 Jun 2020 15:15:51 +0200 Subject: [PATCH 2/9] #99 resolve super calls and other inheritance-related fixes: - add inheritance information also for CXX files - split supertypes into superclasses and implemented interfaces -> needed for correct Java super call resolving - automatically add "this" field to dummy records as well - correctly resolve super.call() and BaseName.super.call() according to JLS 13 - unify handling of the dedicated "unknown declarations" translation unit - remove transitive closure for inheritance --- .../cpg/frontends/cpp/DeclaratorHandler.java | 23 ++--- .../frontends/java/DeclarationHandler.java | 28 +++--- .../aisec/cpg/graph/NodeBuilder.java | 19 +++- .../aisec/cpg/graph/RecordDeclaration.java | 52 ++++++++--- .../aisec/cpg/passes/CallResolver.java | 92 ++++++++++++++++++- .../aisec/cpg/passes/ImportResolver.java | 11 +-- .../de/fraunhofer/aisec/cpg/passes/Pass.java | 29 ++++-- .../cpg/passes/TypeHierarchyResolver.java | 51 +++++----- .../cpg/passes/VariableUsageResolver.java | 61 ++++++------ 9 files changed, 252 insertions(+), 114 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/DeclaratorHandler.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/DeclaratorHandler.java index ddf87a5c678..921cbb8aa8b 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/DeclaratorHandler.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/cpp/DeclaratorHandler.java @@ -32,10 +32,11 @@ import de.fraunhofer.aisec.cpg.graph.type.UnknownType; import de.fraunhofer.aisec.cpg.passes.scopes.Scope; import java.lang.reflect.Method; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.eclipse.cdt.core.dom.ast.*; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTParameterDeclaration; @@ -206,27 +207,17 @@ private RecordDeclaration handleCompositeTypeSpecifier(CPPASTCompositeTypeSpecif RecordDeclaration recordDeclaration = NodeBuilder.newRecordDeclaration( lang.getScopeManager().getCurrentNamePrefixWithDelimiter() + ctx.getName().toString(), - new ArrayList<>(), kind, ctx.getRawSignature()); + recordDeclaration.setSuperClasses( + Arrays.stream(ctx.getBaseSpecifiers()) + .map(b -> TypeParser.createFrom(b.getNameSpecifier().toString(), true)) + .collect(Collectors.toList())); this.lang.addRecord(recordDeclaration); lang.getScopeManager().enterScope(recordDeclaration); - - if (kind.equals("class")) { - de.fraunhofer.aisec.cpg.graph.FieldDeclaration thisDeclaration = - NodeBuilder.newFieldDeclaration( - "this", - TypeParser.createFrom(ctx.getName().toString(), true), - new ArrayList<>(), - "this", - null, - null, - true); - recordDeclaration.getFields().add(thisDeclaration); - lang.getScopeManager().addValueDeclaration(thisDeclaration); - } + lang.getScopeManager().addValueDeclaration(recordDeclaration.getThis()); for (IASTDeclaration member : ctx.getMembers()) { if (member instanceof CPPASTVisibilityLabel) { diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java index d1a6fcb3cd6..07c115ae1a0 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java @@ -218,7 +218,20 @@ public RecordDeclaration handleClassOrInterfaceDeclaration( // add a type declaration RecordDeclaration recordDeclaration = - NodeBuilder.newRecordDeclaration(name, superTypes, "class", classInterDecl.toString()); + NodeBuilder.newRecordDeclaration(name, "class", classInterDecl.toString()); + recordDeclaration.setSuperClasses( + classInterDecl.getExtendedTypes().stream() + .map(this.lang::getTypeAsGoodAsPossible) + .collect(Collectors.toList())); + if (recordDeclaration.getSuperClasses().isEmpty()) { + List superClasses = new ArrayList<>(); + superClasses.add(TypeParser.createFrom("java.lang.Object", true)); + recordDeclaration.setSuperClasses(superClasses); + } + recordDeclaration.setImplementedInterfaces( + classInterDecl.getImplementedTypes().stream() + .map(this.lang::getTypeAsGoodAsPossible) + .collect(Collectors.toList())); Map> partitioned = this.lang.getContext().getImports().stream() @@ -240,18 +253,7 @@ public RecordDeclaration handleClassOrInterfaceDeclaration( this.lang.addRecord(recordDeclaration); lang.getScopeManager().enterScope(recordDeclaration); - - de.fraunhofer.aisec.cpg.graph.FieldDeclaration thisDeclaration = - NodeBuilder.newFieldDeclaration( - "this", - TypeParser.createFrom(name, true), - new ArrayList<>(), - "this", - null, - null, - false); - recordDeclaration.getFields().add(thisDeclaration); - lang.getScopeManager().addValueDeclaration(thisDeclaration); + lang.getScopeManager().addValueDeclaration(recordDeclaration.getThis()); // TODO: 'this' identifier for multiple instances? for (BodyDeclaration decl : classInterDecl.getMembers()) { diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/NodeBuilder.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/NodeBuilder.java index f7fbba5d35a..caf2725bd1e 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/NodeBuilder.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/NodeBuilder.java @@ -27,7 +27,9 @@ package de.fraunhofer.aisec.cpg.graph; import de.fraunhofer.aisec.cpg.graph.type.Type; +import de.fraunhofer.aisec.cpg.graph.type.TypeParser; import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation; +import java.util.ArrayList; import java.util.List; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -395,14 +397,25 @@ public static TranslationUnitDeclaration newTranslationUnitDeclaration(String na return node; } - public static RecordDeclaration newRecordDeclaration( - String name, List superTypes, String kind, String code) { + public static RecordDeclaration newRecordDeclaration(String name, String kind, String code) { RecordDeclaration node = new RecordDeclaration(); node.setName(name); - node.setSuperTypes(superTypes); node.setKind(kind); node.setCode(code); + if (kind.equals("class")) { + de.fraunhofer.aisec.cpg.graph.FieldDeclaration thisDeclaration = + NodeBuilder.newFieldDeclaration( + "this", + TypeParser.createFrom(name, true), + new ArrayList<>(), + "this", + null, + null, + true); + node.getFields().add(thisDeclaration); + } + log(node); return node; diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java index a6a2d6fa05e..2e4ed638683 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/RecordDeclaration.java @@ -28,8 +28,11 @@ import de.fraunhofer.aisec.cpg.graph.type.Type; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.commons.lang3.builder.ToStringBuilder; import org.checkerframework.checker.nullness.qual.NonNull; +import org.neo4j.ogm.annotation.Transient; /** Represents a C++ union/struct/class or Java class */ public class RecordDeclaration extends Declaration { @@ -49,7 +52,8 @@ public class RecordDeclaration extends Declaration { @SubGraph("AST") private List records = new ArrayList<>(); - private List superTypes = new ArrayList<>(); + @Transient private List superClasses = new ArrayList<>(); + @Transient private List implementedInterfaces = new ArrayList<>(); @org.neo4j.ogm.annotation.Relationship private Set superTypeDeclarations = new HashSet<>(); @@ -91,12 +95,6 @@ public FieldDeclaration getThis() { return fields.stream().filter(f -> f.getName().equals("this")).findFirst().orElse(null); } - public FieldDeclaration getSuper() { - Optional superType = - getSuperTypeDeclarations().stream().filter(r -> r.getKind().equals("class")).findFirst(); - return superType.map(RecordDeclaration::getThis).orElse(null); - } - public List getMethods() { return methods; } @@ -121,12 +119,43 @@ public void setRecords(List records) { this.records = records; } + /** + * Combines both implemented interfaces and extended classes. This is most commonly what you are + * looking for when looking for method call targets etc. + * + * @return concatenation of {@link #getSuperClasses()} and {@link #getImplementedInterfaces()} + */ public List getSuperTypes() { - return superTypes; + return Stream.of(superClasses, implementedInterfaces) + .flatMap(Collection::stream) + .collect(Collectors.toList()); + } + + /** + * The classes that are extended by this one. Usually zero or one, but in C++ this can contain + * multiple classes + * + * @return extended classes + */ + public List getSuperClasses() { + return superClasses; + } + + public void setSuperClasses(List superClasses) { + this.superClasses = superClasses; + } + + /** + * Interfaces implemented by this class. This concept is not present in C++ + * + * @return the list of implemented interfaces + */ + public List getImplementedInterfaces() { + return implementedInterfaces; } - public void setSuperTypes(List superTypes) { - this.superTypes = superTypes; + public void setImplementedInterfaces(List implementedInterfaces) { + this.implementedInterfaces = implementedInterfaces; } public Set getSuperTypeDeclarations() { @@ -198,7 +227,8 @@ public boolean equals(Object o) { && Objects.equals(methods, that.methods) && Objects.equals(constructors, that.constructors) && Objects.equals(records, that.records) - && Objects.equals(superTypes, that.superTypes) + && Objects.equals(superClasses, that.superClasses) + && Objects.equals(implementedInterfaces, that.implementedInterfaces) && Objects.equals(superTypeDeclarations, that.superTypeDeclarations); } 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 4c87a13241b..1dff28474a1 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java @@ -27,13 +27,44 @@ package de.fraunhofer.aisec.cpg.passes; import de.fraunhofer.aisec.cpg.TranslationResult; -import de.fraunhofer.aisec.cpg.graph.*; +import de.fraunhofer.aisec.cpg.frontends.java.JavaLanguageFrontend; +import de.fraunhofer.aisec.cpg.graph.CallExpression; +import de.fraunhofer.aisec.cpg.graph.ConstructExpression; +import de.fraunhofer.aisec.cpg.graph.ConstructorDeclaration; +import de.fraunhofer.aisec.cpg.graph.DeclaredReferenceExpression; +import de.fraunhofer.aisec.cpg.graph.ExplicitConstructorInvocation; +import de.fraunhofer.aisec.cpg.graph.Expression; +import de.fraunhofer.aisec.cpg.graph.FunctionDeclaration; +import de.fraunhofer.aisec.cpg.graph.HasType; +import de.fraunhofer.aisec.cpg.graph.MemberCallExpression; +import de.fraunhofer.aisec.cpg.graph.MethodDeclaration; +import de.fraunhofer.aisec.cpg.graph.NewExpression; +import de.fraunhofer.aisec.cpg.graph.Node; +import de.fraunhofer.aisec.cpg.graph.NodeBuilder; +import de.fraunhofer.aisec.cpg.graph.ParamVariableDeclaration; +import de.fraunhofer.aisec.cpg.graph.RecordDeclaration; +import de.fraunhofer.aisec.cpg.graph.StaticCallExpression; +import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; +import de.fraunhofer.aisec.cpg.graph.ValueDeclaration; +import de.fraunhofer.aisec.cpg.graph.VariableDeclaration; import de.fraunhofer.aisec.cpg.graph.type.FunctionPointerType; import de.fraunhofer.aisec.cpg.graph.type.Type; import de.fraunhofer.aisec.cpg.graph.type.TypeParser; import de.fraunhofer.aisec.cpg.helpers.SubgraphWalker.ScopedWalker; import de.fraunhofer.aisec.cpg.helpers.Util; -import java.util.*; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Deque; +import java.util.HashMap; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; import org.checkerframework.checker.nullness.qual.NonNull; @@ -149,6 +180,56 @@ private void fixInitializers(@NonNull Node node, RecordDeclaration curClass) { } } + /** + * Handle calls in the form of super.call() or ClassName.super.call() + * , conforming to JLS13 ยง15.12.1 + * + * @param curClass The class containing the call + * @param call The call to be resolved + */ + private void handleSuperCall(RecordDeclaration curClass, CallExpression call) { + RecordDeclaration target = null; + if (call.getBase().getName().equals("super")) { + // direct superclass, either defined explicitly or java.lang.Object by default + if (!curClass.getSuperClasses().isEmpty()) { + target = recordMap.get(curClass.getSuperClasses().get(0).getTypeName()); + } else { + Util.warnWithFileLocation( + call, + LOGGER, + "super call without direct superclass! Expected " + + "java.lang.Object to be present at least!"); + } + } else { + // BaseName.super.call(), might either be in order to specify an enclosing class or an + // interface that is implemented + String baseName = + call.getBase().getName().substring(0, call.getBase().getName().lastIndexOf(".super")); + if (curClass.getImplementedInterfaces().contains(TypeParser.createFrom(baseName, true))) { + // Basename is an interface -> BaseName.super refers to BaseName itself + target = recordMap.get(baseName); + } else { + // BaseName refers to an enclosing class -> BaseName.super is BaseName's superclass + RecordDeclaration base = recordMap.get(baseName); + if (base != null) { + if (!base.getSuperClasses().isEmpty()) { + target = recordMap.get(base.getSuperClasses().get(0).getTypeName()); + } else { + Util.warnWithFileLocation( + call, + LOGGER, + "super call without direct superclass! Expected " + + "java.lang.Object to be present at least!"); + } + } + } + } + if (target != null) { + ((DeclaredReferenceExpression) call.getBase()).setRefersTo(target.getThis()); + handleMethodCall(target, call); + } + } + private void resolve(@NonNull Node node, RecordDeclaration curClass) { if (node instanceof TranslationUnitDeclaration) { this.currentTU = (TranslationUnitDeclaration) node; @@ -157,6 +238,13 @@ private void resolve(@NonNull Node node, RecordDeclaration curClass) { } else if (node instanceof CallExpression) { CallExpression call = (CallExpression) node; + if (lang instanceof JavaLanguageFrontend + && call.getBase() instanceof DeclaredReferenceExpression + && call.getBase().getName().matches("(?.+\\.)?super")) { + handleSuperCall(curClass, call); + return; + } + if (call instanceof MemberCallExpression) { Node member = ((MemberCallExpression) call).getMember(); if (member instanceof HasType diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/ImportResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/ImportResolver.java index ba7a7988764..9e2d2aec646 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/ImportResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/ImportResolver.java @@ -75,12 +75,8 @@ public void accept(TranslationResult result) { if (!unknownTypes.isEmpty()) { // Get the translation unit holding all unknown declarations, or create a new one if necessary - TranslationUnitDeclaration unknownDeclarations = - result.getTranslationUnits().stream() - .filter(tu -> tu.getName().equals("unknown declarations")) - .findFirst() - .orElseGet(() -> createUnknownTranslationUnit(result)); - unknownDeclarations.setDeclarations(new ArrayList<>(unknownTypes.values())); + TranslationUnitDeclaration unknownDeclarations = getUnknownDeclarationsTU(result); + unknownDeclarations.getDeclarations().addAll(unknownTypes.values()); importables.putAll(unknownTypes); } } @@ -142,8 +138,7 @@ private Declaration getOrCreateDeclaration(String name) { } else { // Create stubs for unknown imports if (!unknownTypes.containsKey(name)) { - unknownTypes.put( - name, NodeBuilder.newRecordDeclaration(name, Collections.emptyList(), "class", "")); + unknownTypes.put(name, NodeBuilder.newRecordDeclaration(name, "class", "")); } return unknownTypes.get(name); } diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/Pass.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/Pass.java index 27e4c48ca86..391bcd57720 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/Pass.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/Pass.java @@ -30,6 +30,7 @@ import de.fraunhofer.aisec.cpg.frontends.LanguageFrontend; import de.fraunhofer.aisec.cpg.graph.NodeBuilder; import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; +import java.util.Optional; import java.util.function.Consumer; import org.checkerframework.checker.nullness.qual.Nullable; @@ -59,12 +60,28 @@ public void setLang(@Nullable LanguageFrontend lang) { public abstract void cleanup(); - TranslationUnitDeclaration createUnknownTranslationUnit(TranslationResult result) { - TranslationUnitDeclaration declaration = - NodeBuilder.newTranslationUnitDeclaration("unknown declarations", ""); - declaration.setImplicit(true); - result.getTranslationUnits().add(declaration); - return declaration; + /** + * Get the {@link TranslationUnitDeclaration} that is to be used for grouping unknown + * declarations, e.g. dummy {@link de.fraunhofer.aisec.cpg.graph.RecordDeclaration} nodes. This TU + * should be unique, thus this method ensures that a new one is only created if needed. + * + * @param result The {@link TranslationResult} that should contain this translation unit + * @return The {@link TranslationUnitDeclaration} used for dummy declarations + */ + TranslationUnitDeclaration getUnknownDeclarationsTU(TranslationResult result) { + Optional unknownDeclarations = + result.getTranslationUnits().stream() + .filter(tu -> tu.getName().equals("unknown declarations")) + .findFirst(); + if (unknownDeclarations.isPresent()) { + return unknownDeclarations.get(); + } else { + TranslationUnitDeclaration declaration = + NodeBuilder.newTranslationUnitDeclaration("unknown declarations", ""); + declaration.setImplicit(true); + result.getTranslationUnits().add(declaration); + return declaration; + } } /** diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/TypeHierarchyResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/TypeHierarchyResolver.java index 916695ae897..b9e994a81e1 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/TypeHierarchyResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/TypeHierarchyResolver.java @@ -28,11 +28,21 @@ import de.fraunhofer.aisec.cpg.TranslationResult; import de.fraunhofer.aisec.cpg.frontends.LanguageFrontend; -import de.fraunhofer.aisec.cpg.graph.*; -import de.fraunhofer.aisec.cpg.graph.type.Type; -import de.fraunhofer.aisec.cpg.graph.type.TypeParser; +import de.fraunhofer.aisec.cpg.graph.EnumDeclaration; +import de.fraunhofer.aisec.cpg.graph.HasType; +import de.fraunhofer.aisec.cpg.graph.MethodDeclaration; +import de.fraunhofer.aisec.cpg.graph.Node; +import de.fraunhofer.aisec.cpg.graph.NodeBuilder; +import de.fraunhofer.aisec.cpg.graph.RecordDeclaration; +import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; import de.fraunhofer.aisec.cpg.helpers.SubgraphWalker; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; /** @@ -90,12 +100,8 @@ public void accept(TranslationResult translationResult) { if (!unknownTypes.isEmpty()) { // Get the translation unit holding all unknown declarations, or create a new one if necessary - TranslationUnitDeclaration unknownDeclarations = - translationResult.getTranslationUnits().stream() - .filter(tu -> tu.getName().equals("unknown declarations")) - .findFirst() - .orElseGet(() -> createUnknownTranslationUnit(translationResult)); - unknownDeclarations.setDeclarations(new ArrayList<>(unknownTypes.values())); + TranslationUnitDeclaration unknownDeclarations = getUnknownDeclarationsTU(translationResult); + unknownDeclarations.getDeclarations().addAll(unknownTypes.values()); recordMap.putAll(unknownTypes); } @@ -131,7 +137,7 @@ private List getAllMethodsFromSupertypes( } private Set findSupertypeRecords(RecordDeclaration record) { - Set localSuperTypeDeclarations = + Set superTypeDeclarations = record.getSuperTypes().stream() .map( t -> { @@ -139,29 +145,18 @@ private Set findSupertypeRecords(RecordDeclaration record) { return recordMap.get(t.getTypeName()); } else { if (!unknownTypes.containsKey(t.getTypeName())) { - unknownTypes.put( - t.getTypeName(), - NodeBuilder.newRecordDeclaration( - t.getTypeName(), Collections.emptyList(), "class", "")); + RecordDeclaration dummy = + NodeBuilder.newRecordDeclaration(t.getTypeName(), "class", ""); + dummy.setImplicit(true); + unknownTypes.put(t.getTypeName(), dummy); } return unknownTypes.get(t.getTypeName()); } }) .collect(Collectors.toSet()); - HashSet allSupertypeRecords = new HashSet<>(localSuperTypeDeclarations); - for (RecordDeclaration superType : localSuperTypeDeclarations) { - allSupertypeRecords.addAll(findSupertypeRecords(superType)); - } - record.setSuperTypeDeclarations(allSupertypeRecords); - List superTypeNames = - allSupertypeRecords.stream() - .map(RecordDeclaration::getName) - .map(t -> TypeParser.createFrom(t, true)) - .distinct() - .collect(Collectors.toList()); - record.setSuperTypes(superTypeNames); - return allSupertypeRecords; + record.setSuperTypeDeclarations(superTypeDeclarations); + return superTypeDeclarations; } private void analyzeOverridingMethods( 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 5aad52ab6f3..effbe8b8adc 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -51,7 +51,6 @@ import de.fraunhofer.aisec.cpg.graph.type.UnknownType; import de.fraunhofer.aisec.cpg.helpers.SubgraphWalker.ScopedWalker; import de.fraunhofer.aisec.cpg.helpers.Util; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -90,6 +89,7 @@ public class VariableUsageResolver extends Pass { private static final Logger log = LoggerFactory.getLogger(VariableUsageResolver.class); private Map> superTypesMap = new HashMap<>(); private Map recordMap = new HashMap<>(); + private Set unknownRecords = new HashSet<>(); private Map enumMap = new HashMap<>(); private TranslationUnitDeclaration currTu; private ScopedWalker walker; @@ -133,6 +133,10 @@ public void accept(TranslationResult result) { walker.registerHandler(this::resolveLocalVarUsage); walker.iterate(tu); } + if (!unknownRecords.isEmpty()) { + TranslationUnitDeclaration unknownDeclarations = getUnknownDeclarationsTU(result); + unknownDeclarations.getDeclarations().addAll(unknownRecords); + } } private void findRecordsAndEnums(Node node, RecordDeclaration curClass) { @@ -266,7 +270,13 @@ private void resolveFieldUsages(Node current, RecordDeclaration curClass) { Node member = memberExpression.getMember(); if (base instanceof DeclaredReferenceExpression) { if (lang instanceof JavaLanguageFrontend && base.getName().equals("super")) { - base = curClass.getSuper(); + if (curClass != null && curClass.getSuperClasses().size() > 0) { + base = recordMap.get(curClass.getSuperClasses().get(0)).getThis(); + } else { + // no explicit super type -> java.lang.Object + Type objectType = TypeParser.createFrom("java.lang.Object", true); + base = handleUnknownField(objectType, "this", objectType); + } } else { base = resolveBase((DeclaredReferenceExpression) memberExpression.getBase()); } @@ -341,7 +351,7 @@ private Declaration resolveBase(DeclaredReferenceExpression reference) { log.info( "Type declaration for {} not found in graph, using dummy to collect all " + "usages", reference.getType()); - return handleUnknownField(reference.getType(), reference); + return handleUnknownField(reference.getType(), reference.getName(), reference.getType()); } } @@ -371,31 +381,27 @@ private ValueDeclaration resolveMember( .findFirst(); } // Attention: using orElse instead of orElseGet will always invoke unknown declaration handling! - return member.orElseGet(() -> handleUnknownField(containingClass, reference)); + return member.orElseGet( + () -> handleUnknownField(containingClass, reference.getName(), reference.getType())); } - private FieldDeclaration handleUnknownField(Type base, DeclaredReferenceExpression reference) { - recordMap.putIfAbsent( - base, - NodeBuilder.newRecordDeclaration( - base.getTypeName(), - new ArrayList<>(), - Type.UNKNOWN_TYPE_STRING, - Type.UNKNOWN_TYPE_STRING)); + private FieldDeclaration handleUnknownField(Type base, String name, Type type) { + if (!recordMap.containsKey(base)) { + RecordDeclaration dummy = + NodeBuilder.newRecordDeclaration( + base.getTypeName(), Type.UNKNOWN_TYPE_STRING, Type.UNKNOWN_TYPE_STRING); + dummy.setImplicit(true); + recordMap.put(base, dummy); + unknownRecords.add(dummy); + } // fields.putIfAbsent(base, new ArrayList<>()); List declarations = recordMap.get(base).getFields(); Optional target = - declarations.stream().filter(f -> f.getName().equals(reference.getName())).findFirst(); + declarations.stream().filter(f -> f.getName().equals(name)).findFirst(); if (target.isEmpty()) { FieldDeclaration declaration = NodeBuilder.newFieldDeclaration( - reference.getName(), - reference.getType(), - Collections.emptyList(), - "", - null, - null, - false); + name, type, Collections.emptyList(), "", null, null, false); declarations.add(declaration); declaration.setImplicit(true); // lang.getScopeManager().addValueDeclaration(declaration); @@ -406,13 +412,14 @@ private FieldDeclaration handleUnknownField(Type base, DeclaredReferenceExpressi } private MethodDeclaration handleUnknownClassMethod(Type base, String name, Type type) { - recordMap.putIfAbsent( - base, - NodeBuilder.newRecordDeclaration( - base.getTypeName(), - new ArrayList<>(), - Type.UNKNOWN_TYPE_STRING, - Type.UNKNOWN_TYPE_STRING)); + if (!recordMap.containsKey(base)) { + RecordDeclaration dummy = + NodeBuilder.newRecordDeclaration( + base.getTypeName(), Type.UNKNOWN_TYPE_STRING, Type.UNKNOWN_TYPE_STRING); + dummy.setImplicit(true); + recordMap.put(base, dummy); + unknownRecords.add(dummy); + } RecordDeclaration containingRecord = recordMap.get(base); List declarations = containingRecord.getMethods(); Optional target = From d26ca7e572116e1bd0b5305f76d4b6a6b15e5e05 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 4 Jun 2020 15:37:14 +0200 Subject: [PATCH 3/9] Adapt java-vs-cpp test: Java classes have implicit java.lang.Object inheritance, leading to a corresponding dummy record declaration --- .../fraunhofer/aisec/cpg/enhancements/JavaVsCppTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 86aff412435..2c2670364a0 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/JavaVsCppTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/JavaVsCppTest.java @@ -64,8 +64,11 @@ private void analyzeAndSave(String pathname) throws ExecutionException, Interrup .build()) .build(); TranslationResult res = analyzer.analyze().get(); - assertEquals(1, res.getTranslationUnits().size()); - TranslationUnitDeclaration tu = res.getTranslationUnits().get(0); + TranslationUnitDeclaration tu = + res.getTranslationUnits().stream() + .filter(t -> !t.getName().equals("unknown declarations")) + .findFirst() + .orElseThrow(); assertEquals(1, tu.getDeclarations().size()); Declaration decl = tu.getDeclarations().get(0); assertTrue(decl instanceof RecordDeclaration); From b4908bfbdfed5bc267f4baf78af857e543a362a0 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 4 Jun 2020 16:33:12 +0200 Subject: [PATCH 4/9] #99 add tests --- .../frontends/java/JavaLanguageFrontend.java | 2 +- .../de/fraunhofer/aisec/cpg/TestUtils.java | 3 +- .../aisec/cpg/enhancements/SuperCallTest.java | 108 ++++++++++++++++++ src/test/resources/superCalls/Interface1.java | 3 + src/test/resources/superCalls/Interface2.java | 3 + src/test/resources/superCalls/SubClass.java | 23 ++++ src/test/resources/superCalls/SuperClass.java | 4 + 7 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java create mode 100644 src/test/resources/superCalls/Interface1.java create mode 100644 src/test/resources/superCalls/Interface2.java create mode 100644 src/test/resources/superCalls/SubClass.java create mode 100644 src/test/resources/superCalls/SuperClass.java 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 8cc82aaf193..2035ecb07c3 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 @@ -187,7 +187,7 @@ protected CompilationUnit parse(File file, JavaParser parser) } log.error(sb.toString()); }); - log.error("Could not parse the file correctly! AST may be empty"); + log.error("Could not parse the file {} correctly! AST may be empty", file); } return optional.get(); } diff --git a/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java b/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java index 2d0044195a1..8416af6810f 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/TestUtils.java @@ -50,8 +50,7 @@ public static S findByPredicate(Collection nodes, Predicate< } public static S findByUniqueName(Collection nodes, String name) { - List results = - nodes.stream().filter(m -> m.getName().equals(name)).collect(Collectors.toList()); + List results = findByName(nodes, name); assertEquals(1, results.size()); return results.get(0); } diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java new file mode 100644 index 00000000000..b44862997da --- /dev/null +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java @@ -0,0 +1,108 @@ +package de.fraunhofer.aisec.cpg.enhancements; + +import static org.junit.jupiter.api.Assertions.*; + +import de.fraunhofer.aisec.cpg.TestUtils; +import de.fraunhofer.aisec.cpg.graph.*; +import de.fraunhofer.aisec.cpg.helpers.Util; +import java.nio.file.Path; +import java.util.List; +import org.junit.jupiter.api.Test; + +public class SuperCallTest { + + private final Path topLevel = Path.of("src", "test", "resources", "superCalls"); + + @Test + void testSimpleCall() throws Exception { + List result = TestUtils.analyze("java", topLevel); + List records = Util.subnodesOfType(result, RecordDeclaration.class); + + RecordDeclaration superClass = TestUtils.findByUniqueName(records, "SuperClass"); + List superMethods = Util.subnodesOfType(superClass, MethodDeclaration.class); + MethodDeclaration superTarget = TestUtils.findByUniqueName(superMethods, "target"); + + RecordDeclaration subClass = TestUtils.findByUniqueName(records, "SubClass"); + List methods = Util.subnodesOfType(subClass, MethodDeclaration.class); + MethodDeclaration target = TestUtils.findByUniqueName(methods, "target"); + List calls = Util.subnodesOfType(target, CallExpression.class); + CallExpression superCall = + TestUtils.findByPredicate(calls, c -> "super.target();".equals(c.getCode())); + + assertEquals(List.of(superTarget), superCall.getInvokes()); + } + + @Test + void testInterfaceCall() throws Exception { + List result = TestUtils.analyze("java", topLevel); + List records = Util.subnodesOfType(result, RecordDeclaration.class); + + RecordDeclaration interface1 = TestUtils.findByUniqueName(records, "Interface1"); + List interface1Methods = + Util.subnodesOfType(interface1, MethodDeclaration.class); + MethodDeclaration interface1Target = TestUtils.findByUniqueName(interface1Methods, "target"); + + RecordDeclaration interface2 = TestUtils.findByUniqueName(records, "Interface2"); + List interface2Methods = + Util.subnodesOfType(interface2, MethodDeclaration.class); + MethodDeclaration interface2Target = TestUtils.findByUniqueName(interface2Methods, "target"); + + RecordDeclaration subClass = TestUtils.findByUniqueName(records, "SubClass"); + List methods = Util.subnodesOfType(subClass, MethodDeclaration.class); + MethodDeclaration target = TestUtils.findByUniqueName(methods, "target"); + List calls = Util.subnodesOfType(target, CallExpression.class); + CallExpression interface1Call = + TestUtils.findByPredicate(calls, c -> "Interface1.super.target();".equals(c.getCode())); + CallExpression interface2Call = + TestUtils.findByPredicate(calls, c -> "Interface2.super.target();".equals(c.getCode())); + + assertEquals(List.of(interface1Target), interface1Call.getInvokes()); + assertEquals(List.of(interface2Target), interface2Call.getInvokes()); + } + + @Test + void testSuperField() throws Exception { + List result = TestUtils.analyze("java", topLevel); + List records = Util.subnodesOfType(result, RecordDeclaration.class); + + RecordDeclaration superClass = TestUtils.findByUniqueName(records, "SuperClass"); + FieldDeclaration superField = TestUtils.findByUniqueName(superClass.getFields(), "field"); + + RecordDeclaration subClass = TestUtils.findByUniqueName(records, "SubClass"); + List methods = Util.subnodesOfType(subClass, MethodDeclaration.class); + FieldDeclaration field = TestUtils.findByUniqueName(subClass.getFields(), "field"); + + MethodDeclaration getField = TestUtils.findByUniqueName(methods, "getField"); + List refs = Util.subnodesOfType(getField, MemberExpression.class); + MemberExpression fieldRef = TestUtils.findByPredicate(refs, r -> "field".equals(r.getCode())); + + MethodDeclaration getSuperField = TestUtils.findByUniqueName(methods, "getSuperField"); + refs = Util.subnodesOfType(getSuperField, MemberExpression.class); + MemberExpression superFieldRef = + TestUtils.findByPredicate(refs, r -> "super.field".equals(r.getCode())); + + assertEquals(subClass.getThis(), fieldRef.getBase()); + assertEquals(field, fieldRef.getMember()); + assertEquals(superClass.getThis(), superFieldRef.getBase()); + assertEquals(superField, superFieldRef.getMember()); + } + + @Test + void testInnerCall() throws Exception { + List result = TestUtils.analyze("java", topLevel); + List records = Util.subnodesOfType(result, RecordDeclaration.class); + + RecordDeclaration superClass = TestUtils.findByUniqueName(records, "SuperClass"); + List superMethods = Util.subnodesOfType(superClass, MethodDeclaration.class); + MethodDeclaration superTarget = TestUtils.findByUniqueName(superMethods, "target"); + + RecordDeclaration innerClass = TestUtils.findByUniqueName(records, "SubClass.Inner"); + List methods = Util.subnodesOfType(innerClass, MethodDeclaration.class); + MethodDeclaration target = TestUtils.findByUniqueName(methods, "inner"); + List calls = Util.subnodesOfType(target, CallExpression.class); + CallExpression superCall = + TestUtils.findByPredicate(calls, c -> "SubClass.super.target();".equals(c.getCode())); + + assertEquals(List.of(superTarget), superCall.getInvokes()); + } +} diff --git a/src/test/resources/superCalls/Interface1.java b/src/test/resources/superCalls/Interface1.java new file mode 100644 index 00000000000..6d93999ad33 --- /dev/null +++ b/src/test/resources/superCalls/Interface1.java @@ -0,0 +1,3 @@ +public interface Interface1 { + default void target() {} +} \ No newline at end of file diff --git a/src/test/resources/superCalls/Interface2.java b/src/test/resources/superCalls/Interface2.java new file mode 100644 index 00000000000..b3f5b9680f0 --- /dev/null +++ b/src/test/resources/superCalls/Interface2.java @@ -0,0 +1,3 @@ +public interface Interface2 { + default void target() {} +} \ No newline at end of file diff --git a/src/test/resources/superCalls/SubClass.java b/src/test/resources/superCalls/SubClass.java new file mode 100644 index 00000000000..0413548da05 --- /dev/null +++ b/src/test/resources/superCalls/SubClass.java @@ -0,0 +1,23 @@ +public class SubClass extends SuperClass implements Interface1, Interface2 { + public int field; + @Override + public void target() { + super.target(); // SuperClass.target() + Interface1.super.target(); // Interface1.target() + Interface2.super.target(); // Interface2.target() + } + + public int getField() { + return field; + } + + public int getSuperField() { + return super.field; // SuperClass.field + } + + private class Inner { + public void inner() { + SubClass.super.target(); // SuperClass.target() + } + } +} \ No newline at end of file diff --git a/src/test/resources/superCalls/SuperClass.java b/src/test/resources/superCalls/SuperClass.java new file mode 100644 index 00000000000..09613c7d944 --- /dev/null +++ b/src/test/resources/superCalls/SuperClass.java @@ -0,0 +1,4 @@ +public class SuperClass { + public int field; + public void target() {} +} \ No newline at end of file From a0ccc57141ab45385ba1ff9cfec3a570fec68e78 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Fri, 5 Jun 2020 12:08:35 +0200 Subject: [PATCH 5/9] #99 remove wrong "super" fields, add test case for this --- .../frontends/java/DeclarationHandler.java | 7 ----- .../cpg/passes/VariableUsageResolver.java | 28 +++---------------- .../aisec/cpg/enhancements/SuperCallTest.java | 25 +++++++++++++++++ 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java index 07c115ae1a0..5200c44bef8 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java @@ -48,7 +48,6 @@ import de.fraunhofer.aisec.cpg.sarif.PhysicalLocation; import java.util.*; import java.util.stream.Collectors; -import java.util.stream.Stream; public class DeclarationHandler extends Handler { @@ -210,12 +209,6 @@ public RecordDeclaration handleClassOrInterfaceDeclaration( // } name = getAbsoluteName(name); - List superTypes = - Stream.of(classInterDecl.getExtendedTypes(), classInterDecl.getImplementedTypes()) - .flatMap(Collection::stream) - .map(this.lang::getTypeAsGoodAsPossible) - .collect(Collectors.toList()); - // add a type declaration RecordDeclaration recordDeclaration = NodeBuilder.newRecordDeclaration(name, "class", classInterDecl.toString()); 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 effbe8b8adc..82a0281333d 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -30,35 +30,14 @@ import de.fraunhofer.aisec.cpg.TranslationResult; import de.fraunhofer.aisec.cpg.frontends.java.JavaLanguageFrontend; -import de.fraunhofer.aisec.cpg.graph.Declaration; -import de.fraunhofer.aisec.cpg.graph.DeclaredReferenceExpression; -import de.fraunhofer.aisec.cpg.graph.EnumDeclaration; -import de.fraunhofer.aisec.cpg.graph.FieldDeclaration; -import de.fraunhofer.aisec.cpg.graph.FunctionDeclaration; -import de.fraunhofer.aisec.cpg.graph.HasType; -import de.fraunhofer.aisec.cpg.graph.MemberCallExpression; -import de.fraunhofer.aisec.cpg.graph.MemberExpression; -import de.fraunhofer.aisec.cpg.graph.MethodDeclaration; -import de.fraunhofer.aisec.cpg.graph.Node; -import de.fraunhofer.aisec.cpg.graph.NodeBuilder; -import de.fraunhofer.aisec.cpg.graph.RecordDeclaration; -import de.fraunhofer.aisec.cpg.graph.StaticReferenceExpression; -import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; -import de.fraunhofer.aisec.cpg.graph.ValueDeclaration; +import de.fraunhofer.aisec.cpg.graph.*; import de.fraunhofer.aisec.cpg.graph.type.FunctionPointerType; import de.fraunhofer.aisec.cpg.graph.type.Type; import de.fraunhofer.aisec.cpg.graph.type.TypeParser; import de.fraunhofer.aisec.cpg.graph.type.UnknownType; import de.fraunhofer.aisec.cpg.helpers.SubgraphWalker.ScopedWalker; import de.fraunhofer.aisec.cpg.helpers.Util; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -357,7 +336,8 @@ private Declaration resolveBase(DeclaredReferenceExpression reference) { private ValueDeclaration resolveMember( Type containingClass, DeclaredReferenceExpression reference) { - if (lang instanceof JavaLanguageFrontend && reference.getName().equals("super")) { + if (lang instanceof JavaLanguageFrontend + && reference.getName().matches("(?.+\\.)?super")) { // if we have a "super" on the member side, this is a member call. We need to resolve this // in the call resolver instead return null; diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java index b44862997da..c6be7d1a7af 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java @@ -7,6 +7,8 @@ import de.fraunhofer.aisec.cpg.helpers.Util; import java.nio.file.Path; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; public class SuperCallTest { @@ -105,4 +107,27 @@ void testInnerCall() throws Exception { assertEquals(List.of(superTarget), superCall.getInvokes()); } + + @Test + void testNoExcessFields() throws Exception { + List result = TestUtils.analyze("java", topLevel); + List records = Util.subnodesOfType(result, RecordDeclaration.class); + + RecordDeclaration superClass = TestUtils.findByUniqueName(records, "SuperClass"); + assertEquals(2, superClass.getFields().size()); + assertEquals( + Set.of("this", "field"), + superClass.getFields().stream().map(Node::getName).collect(Collectors.toSet())); + + RecordDeclaration subClass = TestUtils.findByUniqueName(records, "SubClass"); + assertEquals(2, subClass.getFields().size()); + assertEquals( + Set.of("this", "field"), + subClass.getFields().stream().map(Node::getName).collect(Collectors.toSet())); + + RecordDeclaration inner = TestUtils.findByUniqueName(records, "SubClass.Inner"); + assertEquals(1, inner.getFields().size()); + assertEquals( + Set.of("this"), inner.getFields().stream().map(Node::getName).collect(Collectors.toSet())); + } } From 7a4cc45f1e3380aad12d331017226ad4268fd681 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Fri, 5 Jun 2020 12:26:44 +0200 Subject: [PATCH 6/9] Some refactoring --- .../aisec/cpg/passes/CallResolver.java | 104 +++++++++--------- .../cpg/passes/VariableUsageResolver.java | 2 +- 2 files changed, 56 insertions(+), 50 deletions(-) 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 1dff28474a1..52b2f82a0a0 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java @@ -203,26 +203,7 @@ private void handleSuperCall(RecordDeclaration curClass, CallExpression call) { } else { // BaseName.super.call(), might either be in order to specify an enclosing class or an // interface that is implemented - String baseName = - call.getBase().getName().substring(0, call.getBase().getName().lastIndexOf(".super")); - if (curClass.getImplementedInterfaces().contains(TypeParser.createFrom(baseName, true))) { - // Basename is an interface -> BaseName.super refers to BaseName itself - target = recordMap.get(baseName); - } else { - // BaseName refers to an enclosing class -> BaseName.super is BaseName's superclass - RecordDeclaration base = recordMap.get(baseName); - if (base != null) { - if (!base.getSuperClasses().isEmpty()) { - target = recordMap.get(base.getSuperClasses().get(0).getTypeName()); - } else { - Util.warnWithFileLocation( - call, - LOGGER, - "super call without direct superclass! Expected " - + "java.lang.Object to be present at least!"); - } - } - } + target = handleSpecificSupertype(curClass, call); } if (target != null) { ((DeclaredReferenceExpression) call.getBase()).setRefersTo(target.getThis()); @@ -230,45 +211,70 @@ private void handleSuperCall(RecordDeclaration curClass, CallExpression call) { } } + private RecordDeclaration handleSpecificSupertype(RecordDeclaration curClass, + CallExpression call) { + String baseName = + call.getBase().getName().substring(0, call.getBase().getName().lastIndexOf(".super")); + if (curClass.getImplementedInterfaces().contains(TypeParser.createFrom(baseName, true))) { + // Basename is an interface -> BaseName.super refers to BaseName itself + return recordMap.get(baseName); + } else { + // BaseName refers to an enclosing class -> BaseName.super is BaseName's superclass + RecordDeclaration base = recordMap.get(baseName); + if (base != null) { + if (!base.getSuperClasses().isEmpty()) { + return recordMap.get(base.getSuperClasses().get(0).getTypeName()); + } else { + Util.warnWithFileLocation(call, + LOGGER, + "super call without direct superclass! Expected " + + "java.lang.Object to be present at least!"); + } + } + } + return null; + } + private void resolve(@NonNull Node node, RecordDeclaration curClass) { if (node instanceof TranslationUnitDeclaration) { this.currentTU = (TranslationUnitDeclaration) node; } else if (node instanceof ExplicitConstructorInvocation) { resolveExplicitConstructorInvocation((ExplicitConstructorInvocation) node); } else if (node instanceof CallExpression) { - CallExpression call = (CallExpression) node; + handleCallExpression(curClass, (CallExpression) node); + } else if (node instanceof ConstructExpression) { + resolveConstructExpression((ConstructExpression) node); + } + } - if (lang instanceof JavaLanguageFrontend - && call.getBase() instanceof DeclaredReferenceExpression - && call.getBase().getName().matches("(?.+\\.)?super")) { - handleSuperCall(curClass, call); - return; - } + private void handleCallExpression(RecordDeclaration curClass, CallExpression call) { + if (lang instanceof JavaLanguageFrontend + && call.getBase() instanceof DeclaredReferenceExpression + && call.getBase().getName().matches("(?.+\\.)?super")) { + handleSuperCall(curClass, call); + return; + } - if (call instanceof MemberCallExpression) { - Node member = ((MemberCallExpression) call).getMember(); - if (member instanceof HasType - && ((HasType) member).getType() instanceof FunctionPointerType) { - handleFunctionPointerCall(call, member); - return; - } + if (call instanceof MemberCallExpression) { + Node member = ((MemberCallExpression) call).getMember(); + if (member instanceof HasType + && ((HasType) member).getType() instanceof FunctionPointerType) { + handleFunctionPointerCall(call, member); + return; } + } - // we could be referring to a function pointer even though it is not a member call if the - // usual function pointer syntax (*fp)() has been omitted: fp(). Looks like a normal call, - // but it isn't - Optional funcPointer = - walker.getDeclarationForScope( - call, - v -> - v.getType() instanceof FunctionPointerType && v.getName().equals(call.getName())); - if (funcPointer.isPresent()) { - handleFunctionPointerCall(call, funcPointer.get()); - } else { - handleNormalCalls(curClass, call); - } - } else if (node instanceof ConstructExpression) { - resolveConstructExpression((ConstructExpression) node); + // we could be referring to a function pointer even though it is not a member call if the + // usual function pointer syntax (*fp)() has been omitted: fp(). Looks like a normal call, + // but it isn't + Optional funcPointer = + walker.getDeclarationForScope(call, + v -> + v.getType() instanceof FunctionPointerType && v.getName().equals(call.getName())); + if (funcPointer.isPresent()) { + handleFunctionPointerCall(call, funcPointer.get()); + } else { + handleNormalCalls(curClass, call); } } 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 82a0281333d..0c47868ceaa 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -249,7 +249,7 @@ private void resolveFieldUsages(Node current, RecordDeclaration curClass) { Node member = memberExpression.getMember(); if (base instanceof DeclaredReferenceExpression) { if (lang instanceof JavaLanguageFrontend && base.getName().equals("super")) { - if (curClass != null && curClass.getSuperClasses().size() > 0) { + if (curClass != null && !curClass.getSuperClasses().isEmpty()) { base = recordMap.get(curClass.getSuperClasses().get(0)).getThis(); } else { // no explicit super type -> java.lang.Object From 9cbd3d9301ef017c4f00d134edb2ab70be9839e6 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 18 Jun 2020 15:37:25 +0200 Subject: [PATCH 7/9] Fix hardcoded java.lang.Object --- .../fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java | 2 +- .../de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java index 5200c44bef8..50025dcf2eb 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/DeclarationHandler.java @@ -218,7 +218,7 @@ public RecordDeclaration handleClassOrInterfaceDeclaration( .collect(Collectors.toList())); if (recordDeclaration.getSuperClasses().isEmpty()) { List superClasses = new ArrayList<>(); - superClasses.add(TypeParser.createFrom("java.lang.Object", true)); + superClasses.add(TypeParser.createFrom(Object.class.getName(), true)); recordDeclaration.setSuperClasses(superClasses); } recordDeclaration.setImplementedInterfaces( 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 0c47868ceaa..01737360a11 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -253,7 +253,7 @@ private void resolveFieldUsages(Node current, RecordDeclaration curClass) { base = recordMap.get(curClass.getSuperClasses().get(0)).getThis(); } else { // no explicit super type -> java.lang.Object - Type objectType = TypeParser.createFrom("java.lang.Object", true); + Type objectType = TypeParser.createFrom(Object.class.getName(), true); base = handleUnknownField(objectType, "this", objectType); } } else { From c2c239470faf3e21f90153eecbf82464d154dc16 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Thu, 18 Jun 2020 18:09:56 +0200 Subject: [PATCH 8/9] Apply type parser fix from refersTo-christian branch --- .../de/fraunhofer/aisec/cpg/graph/TypeManager.java | 4 ++++ .../fraunhofer/aisec/cpg/graph/type/TypeParser.java | 5 ++++- .../fraunhofer/aisec/cpg/passes/CallResolver.java | 13 +++++++------ 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/TypeManager.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/TypeManager.java index a0d9cc4dbd5..8843a5c4b6e 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/TypeManager.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/TypeManager.java @@ -269,6 +269,10 @@ public Language getLanguage() { } } + public LanguageFrontend getFrontend() { + return frontend; + } + public boolean isSupertypeOf(Type superType, Type subType) { if (superType.getReferenceDepth() != subType.getReferenceDepth()) { return false; diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/type/TypeParser.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/type/TypeParser.java index 8dd69969615..abbdce058c8 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/type/TypeParser.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/type/TypeParser.java @@ -67,7 +67,10 @@ public static void setLanguage(TypeManager.Language language) { } public static TypeManager.Language getLanguage() { - return language; + if (TypeManager.getInstance().getFrontend() == null) { + return language; + } + return TypeManager.getInstance().getLanguage(); } /** 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 52b2f82a0a0..6a096115362 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java @@ -211,8 +211,8 @@ private void handleSuperCall(RecordDeclaration curClass, CallExpression call) { } } - private RecordDeclaration handleSpecificSupertype(RecordDeclaration curClass, - CallExpression call) { + private RecordDeclaration handleSpecificSupertype( + RecordDeclaration curClass, CallExpression call) { String baseName = call.getBase().getName().substring(0, call.getBase().getName().lastIndexOf(".super")); if (curClass.getImplementedInterfaces().contains(TypeParser.createFrom(baseName, true))) { @@ -225,7 +225,8 @@ private RecordDeclaration handleSpecificSupertype(RecordDeclaration curClass, if (!base.getSuperClasses().isEmpty()) { return recordMap.get(base.getSuperClasses().get(0).getTypeName()); } else { - Util.warnWithFileLocation(call, + Util.warnWithFileLocation( + call, LOGGER, "super call without direct superclass! Expected " + "java.lang.Object to be present at least!"); @@ -268,9 +269,9 @@ private void handleCallExpression(RecordDeclaration curClass, CallExpression cal // usual function pointer syntax (*fp)() has been omitted: fp(). Looks like a normal call, // but it isn't Optional funcPointer = - walker.getDeclarationForScope(call, - v -> - v.getType() instanceof FunctionPointerType && v.getName().equals(call.getName())); + walker.getDeclarationForScope( + call, + v -> v.getType() instanceof FunctionPointerType && v.getName().equals(call.getName())); if (funcPointer.isPresent()) { handleFunctionPointerCall(call, funcPointer.get()); } else { From ea72a451f3a847cded127ab807a93a1372f3e6d0 Mon Sep 17 00:00:00 2001 From: oxisto Date: Thu, 18 Jun 2020 20:31:55 +0200 Subject: [PATCH 9/9] Update src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java --- .../de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java index c6be7d1a7af..6c998e66386 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/SuperCallTest.java @@ -11,7 +11,7 @@ import java.util.stream.Collectors; import org.junit.jupiter.api.Test; -public class SuperCallTest { +class SuperCallTest { private final Path topLevel = Path.of("src", "test", "resources", "superCalls");