From a3d7683ef8227b7ead409f3d50d6d6ecfea874a5 Mon Sep 17 00:00:00 2001 From: zcai Date: Thu, 10 Feb 2022 14:05:15 -0500 Subject: [PATCH 1/4] Stop writing solutions that are equivalent to default annotations (#339) --- .../inference/DefaultSlotManager.java | 143 +++++++++++++- .../InferenceAnnotatedTypeFactory.java | 1 + src/checkers/inference/InferenceLauncher.java | 10 +- src/checkers/inference/InferenceMain.java | 57 ++++-- src/checkers/inference/InferenceOptions.java | 3 + src/checkers/inference/SlotManager.java | 9 + .../inference/model/SourceVariableSlot.java | 30 ++- .../model/serialization/JsonDeserializer.java | 4 +- .../util/SlotDefaultTypeResolver.java | 187 ++++++++++++++++++ 9 files changed, 420 insertions(+), 24 deletions(-) create mode 100644 src/checkers/inference/util/SlotDefaultTypeResolver.java diff --git a/src/checkers/inference/DefaultSlotManager.java b/src/checkers/inference/DefaultSlotManager.java index c5202a0b..a18e4a37 100644 --- a/src/checkers/inference/DefaultSlotManager.java +++ b/src/checkers/inference/DefaultSlotManager.java @@ -1,13 +1,23 @@ package checkers.inference; +import checkers.inference.util.SlotDefaultTypeResolver; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.Symbol; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeFactory; import org.checkerframework.framework.type.AnnotatedTypeMirror; import org.checkerframework.javacutil.AnnotationBuilder; import org.checkerframework.javacutil.AnnotationUtils; import org.checkerframework.javacutil.BugInCF; +import org.checkerframework.javacutil.TypeKindUtils; +import org.checkerframework.javacutil.TypesUtils; import java.lang.annotation.Annotation; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -17,6 +27,9 @@ import javax.annotation.processing.ProcessingEnvironment; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; +import javax.lang.model.element.Element; +import javax.lang.model.element.Name; +import javax.lang.model.element.TypeElement; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; @@ -35,8 +48,8 @@ import checkers.inference.model.SourceVariableSlot; import checkers.inference.model.VariableSlot; import checkers.inference.qual.VarAnnot; -import org.checkerframework.javacutil.TypeKindUtils; -import org.checkerframework.javacutil.TypesUtils; +import scenelib.annotations.io.ASTIndex; +import scenelib.annotations.io.ASTRecord; /** * The default implementation of SlotManager. @@ -46,6 +59,12 @@ public class DefaultSlotManager implements SlotManager { private final AnnotationMirror varAnnot; + /** + * The top annotation in the real qualifier hierarchy. + * Currently, we assume there's only one top. + */ + private final AnnotationMirror realTop; + // This id starts at 1 because in some serializer's // (CnfSerializer) 0 is used as line delimiters. // Monotonically increasing id for all VariableSlots (including @@ -94,6 +113,12 @@ public class DefaultSlotManager implements SlotManager { private final Map, Integer> lubSlotPairCache; private final Map, Integer> glbSlotPairCache; + /** + * A map of tree to {@link AnnotationMirror} for caching + * a set of pre-computed default types for the current compilation unit. + */ + private final Map defaultAnnotationsCache; + /** * A map of {@link AnnotationLocation} to {@link Integer} for caching * {@link ArithmeticVariableSlot}s. The annotation location uniquely identifies an @@ -122,9 +147,11 @@ public class DefaultSlotManager implements SlotManager { private final ProcessingEnvironment processingEnvironment; public DefaultSlotManager( final ProcessingEnvironment processingEnvironment, + final AnnotationMirror realTop, final Set> realQualifiers, boolean storeConstants) { this.processingEnvironment = processingEnvironment; + this.realTop = realTop; // sort the qualifiers so that they are always assigned the same varId this.realQualifiers = sortAnnotationClasses(realQualifiers); slots = new LinkedHashMap<>(); @@ -143,6 +170,7 @@ public DefaultSlotManager( final ProcessingEnvironment processingEnvironment, arithmeticSlotCache = new LinkedHashMap<>(); comparisonThenSlotCache = new LinkedHashMap<>(); comparisonElseSlotCache = new LinkedHashMap<>(); + defaultAnnotationsCache = new LinkedHashMap<>(); if (storeConstants) { for (Class annoClass : this.realQualifiers) { @@ -319,13 +347,38 @@ public int getNumberOfSlots() { return nextId - 1; } + @Override + public void setRoot(CompilationUnitTree compilationUnit) { + this.defaultAnnotationsCache.clear(); + + BaseAnnotatedTypeFactory realTypeFactory = InferenceMain.getInstance().getRealTypeFactory(); + Map defaultTypes = SlotDefaultTypeResolver.resolve( + compilationUnit, + realTypeFactory + ); + + for (Map.Entry entry : defaultTypes.entrySet()) { + // find default types in the current hierarchy and save them to the cache + this.defaultAnnotationsCache.put( + entry.getKey(), + entry.getValue().getAnnotationInHierarchy(this.realTop) + ); + } + } + @Override public SourceVariableSlot createSourceVariableSlot(AnnotationLocation location, TypeMirror type) { + AnnotationMirror defaultAnnotation = null; + if (!InferenceOptions.makeDefaultsExplicit) { + // retrieve the default annotation when needed + defaultAnnotation = getDefaultAnnotationForLocation(location, type); + } + SourceVariableSlot sourceVarSlot; if (location.getKind() == AnnotationLocation.Kind.MISSING) { if (InferenceMain.isHackMode()) { //Don't cache slot for MISSING LOCATION. Just create a new one and return. - sourceVarSlot = new SourceVariableSlot(nextId(), location, type, true); + sourceVarSlot = new SourceVariableSlot(nextId(), location, type, defaultAnnotation, true); addToSlots(sourceVarSlot); } else { throw new BugInCF("Creating SourceVariableSlot on MISSING_LOCATION!"); @@ -335,13 +388,95 @@ public SourceVariableSlot createSourceVariableSlot(AnnotationLocation location, int id = locationCache.get(location); sourceVarSlot = (SourceVariableSlot) getSlot(id); } else { - sourceVarSlot = new SourceVariableSlot(nextId(), location, type, true); + sourceVarSlot = new SourceVariableSlot(nextId(), location, type, defaultAnnotation, true); addToSlots(sourceVarSlot); locationCache.put(location, sourceVarSlot.getId()); } return sourceVarSlot; } + /** + * Find the default annotation for this location by checking the real type factory. + * @param location location to create a new {@link SourceVariableSlot} + * @return the default annotation for the given location + */ + private @Nullable AnnotationMirror getDefaultAnnotationForLocation(AnnotationLocation location, TypeMirror type) { + if (location == AnnotationLocation.MISSING_LOCATION) { + if (InferenceMain.isHackMode()) { + return null; + } else { + throw new BugInCF("Getting default annotation for missing location!"); + } + } + + final Tree tree; // the tree associated with the location + BaseAnnotatedTypeFactory realTypeFactory = InferenceMain.getInstance().getRealTypeFactory(); + + if (location instanceof AnnotationLocation.AstPathLocation) { + tree = getTreeForLocation((AnnotationLocation.AstPathLocation) location); + } else if (location instanceof AnnotationLocation.ClassDeclLocation) { + tree = getTreeForLocation( + (AnnotationLocation.ClassDeclLocation) location, + type, + realTypeFactory + ); + } else { + throw new BugInCF("Unable to find default annotation for location " + location); + } + + AnnotationMirror realAnnotation = null; + if (tree != null) { + realAnnotation = this.defaultAnnotationsCache.get(tree); + if (realAnnotation == null) { + // If its default type can't be found in the cache, we can + // fallback to the simplest method. + realAnnotation = realTypeFactory.getAnnotatedType(tree).getAnnotationInHierarchy(this.realTop); + } + } + return realAnnotation; + } + + /** + * Find the tree associated with the given {@link AnnotationLocation.AstPathLocation}. + * @param location location to find the tree + * @return the tree associated with the given location, which can be null if the location + * is not under the current compilation unit + */ + private @Nullable Tree getTreeForLocation(AnnotationLocation.AstPathLocation location) { + ASTRecord astRecord = location.getAstRecord(); + CompilationUnitTree root = astRecord.ast; + return ASTIndex.getNode(root, astRecord); + } + + /** + * Find the class tree associated with the given {@link AnnotationLocation.ClassDeclLocation}. + * @param realTypeFactory the current real {@link BaseAnnotatedTypeFactory} + * @param location location to find the tree + * @param type type of the associated class + * @return the class tree associated with the given location + */ + private Tree getTreeForLocation( + AnnotationLocation.ClassDeclLocation location, + TypeMirror type, + BaseAnnotatedTypeFactory realTypeFactory + ) { + Element element = processingEnvironment.getTypeUtils().asElement(type); + if (!(element instanceof TypeElement)) { + throw new BugInCF( + "Expected to get a TypeElement for %s at %s, but received %s.", type, location, element); + } + + TypeElement typeElement = (TypeElement) element; + Name fullyQualifiedName = ((Symbol.ClassSymbol)typeElement).flatName(); + if (!fullyQualifiedName.contentEquals(location.getFullyQualifiedClassName())) { + throw new BugInCF( + "TypeElement for %s has qualified name %s, and it doesn't match with the location %s", + type, fullyQualifiedName, location); + } + + return realTypeFactory.declarationFromElement(typeElement); + } + @Override public RefinementVariableSlot createRefinementVariableSlot(AnnotationLocation location, Slot declarationSlot, Slot valueSlot) { // If the location is already cached, return the corresponding refinement slot in the cache diff --git a/src/checkers/inference/InferenceAnnotatedTypeFactory.java b/src/checkers/inference/InferenceAnnotatedTypeFactory.java index 784410e9..ce401e92 100644 --- a/src/checkers/inference/InferenceAnnotatedTypeFactory.java +++ b/src/checkers/inference/InferenceAnnotatedTypeFactory.java @@ -550,6 +550,7 @@ public void setRoot(final CompilationUnitTree root) { compilationUnitsHandled += 1; this.realTypeFactory.setRoot( root ); this.variableAnnotator.clearTreeInfo(); + this.slotManager.setRoot(root); super.setRoot(root); } diff --git a/src/checkers/inference/InferenceLauncher.java b/src/checkers/inference/InferenceLauncher.java index 02661d41..6162bdcb 100644 --- a/src/checkers/inference/InferenceLauncher.java +++ b/src/checkers/inference/InferenceLauncher.java @@ -3,7 +3,6 @@ import org.checkerframework.framework.util.CheckerMain; import org.checkerframework.framework.util.ExecUtil; -import org.checkerframework.javacutil.SystemUtil; import java.io.BufferedReader; import java.io.ByteArrayOutputStream; @@ -190,6 +189,15 @@ public void infer() { addIfTrue("--hacks", InferenceOptions.hacks, argList); + Mode mode = Mode.valueOf(InferenceOptions.mode); + if (InferenceOptions.makeDefaultsExplicit + && (mode == Mode.ROUNDTRIP || mode == Mode.ROUNDTRIP_TYPECHECK)) { + // Two conditions have to be met to make defaults explicit: + // 1. the command-line flag `makeDefaultsExplicit` is provided + // 2. the inference solution will be written back to the source code (roundtrip `mode`) + argList.add("--makeDefaultsExplicit"); + } + argList.add("--"); String compilationBcp = getInferenceCompilationBootclassPath(); diff --git a/src/checkers/inference/InferenceMain.java b/src/checkers/inference/InferenceMain.java index e7d167ac..3e1333a8 100644 --- a/src/checkers/inference/InferenceMain.java +++ b/src/checkers/inference/InferenceMain.java @@ -1,5 +1,7 @@ package checkers.inference; +import checkers.inference.model.SourceVariableSlot; +import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; import java.io.FileOutputStream; @@ -26,6 +28,8 @@ import checkers.inference.qual.VarAnnot; import checkers.inference.util.InferenceUtil; import checkers.inference.util.JaifBuilder; +import org.checkerframework.javacutil.AnnotationUtils; +import org.checkerframework.javacutil.BugInCF; import org.checkerframework.javacutil.SystemUtil; /** @@ -233,21 +237,13 @@ private void writeJaif() { annotationClasses.add(annotation); } } + for (VariableSlot slot : varSlots) { - if (slot.getLocation() != null && slot.isInsertable() - && (solverResult == null || solverResult.containsSolutionForVariable(slot.getId()))) { + if (slot.getLocation() != null) { // TODO: String serialization of annotations. - if (solverResult != null) { - // Not all VariableSlots will have an inferred value. - // This happens for VariableSlots that have no constraints. - AnnotationMirror result = solverResult.getSolutionForVariable(slot.getId()); - if (result != null) { - values.put(slot.getLocation(), result.toString()); - } - } else { - // Just use the VarAnnot in the jaif. - String value = slotManager.getAnnotation(slot).toString(); - values.put(slot.getLocation(), value); + AnnotationMirror annotationToWrite = getAnnotationToWrite(slot); + if (annotationToWrite != null) { + values.put(slot.getLocation(), annotationToWrite.toString()); } } } @@ -285,6 +281,28 @@ private void solve() { } } + private @Nullable AnnotationMirror getAnnotationToWrite(VariableSlot slot) { + if (!slot.isInsertable()) { + return null; + } else if (solverResult == null) { + // Just use the VarAnnot in the jaif. + return slotManager.getAnnotation(slot); + } + + // Not all VariableSlots will have an inferred value. + // This happens for VariableSlots that have no constraints. + AnnotationMirror result = solverResult.getSolutionForVariable(slot.getId()); + if (result != null && slot instanceof SourceVariableSlot) { + AnnotationMirror defaultAnnotation = ((SourceVariableSlot) slot).getDefaultAnnotation(); + + if (defaultAnnotation != null && AnnotationUtils.areSame(defaultAnnotation, result)) { + // Don't need to write a solution that's equivalent to the default annotation. + result = null; + } + } + return result; + } + // ================================================================================ // Component Initialization // ================================================================================ @@ -339,8 +357,17 @@ public BaseAnnotatedTypeFactory getRealTypeFactory() { public SlotManager getSlotManager() { if (slotManager == null ) { - slotManager = new DefaultSlotManager(inferenceChecker.getProcessingEnvironment(), - realTypeFactory.getSupportedTypeQualifiers(), true ); + Set tops = realTypeFactory.getQualifierHierarchy().getTopAnnotations(); + if (tops.size() != 1) { + throw new BugInCF("Expected 1 real top qualifier, but received %d instead", tops.size()); + } + + slotManager = new DefaultSlotManager( + inferenceChecker.getProcessingEnvironment(), + tops.iterator().next(), + realTypeFactory.getSupportedTypeQualifiers(), + true + ); logger.finer("Created slot manager" + slotManager); } return slotManager; diff --git a/src/checkers/inference/InferenceOptions.java b/src/checkers/inference/InferenceOptions.java index 74139adf..3f72dbfb 100644 --- a/src/checkers/inference/InferenceOptions.java +++ b/src/checkers/inference/InferenceOptions.java @@ -100,6 +100,9 @@ public class InferenceOptions { @Option("Additional AFU options") public static String afuOptions; + @Option("If true, insert solutions that are equivalent to the default ones back to the code.") + public static boolean makeDefaultsExplicit; + // ------------------------------------------------------ @OptionGroup("Help") diff --git a/src/checkers/inference/SlotManager.java b/src/checkers/inference/SlotManager.java index ed110340..37cea000 100644 --- a/src/checkers/inference/SlotManager.java +++ b/src/checkers/inference/SlotManager.java @@ -1,6 +1,7 @@ package checkers.inference; import checkers.inference.model.LubVariableSlot; +import com.sun.source.tree.CompilationUnitTree; import org.checkerframework.framework.type.AnnotatedTypeMirror; import java.util.List; @@ -218,4 +219,12 @@ ArithmeticVariableSlot createArithmeticVariableSlot( List getVariableSlots(); List getConstantSlots(); + + /** + * Informs this manager that we are working on a new file, so + * it can preprocess and cache useful information. + * + * @param compilationUnit the current compilation tree + */ + void setRoot(CompilationUnitTree compilationUnit); } diff --git a/src/checkers/inference/model/SourceVariableSlot.java b/src/checkers/inference/model/SourceVariableSlot.java index b1b419ac..95df9f77 100644 --- a/src/checkers/inference/model/SourceVariableSlot.java +++ b/src/checkers/inference/model/SourceVariableSlot.java @@ -1,5 +1,9 @@ package checkers.inference.model; +import checkers.inference.InferenceOptions; +import org.checkerframework.checker.nullness.qual.Nullable; + +import javax.lang.model.element.AnnotationMirror; import javax.lang.model.type.TypeMirror; /** @@ -10,6 +14,13 @@ public class SourceVariableSlot extends VariableSlot { /** The actual type of the type use */ protected final TypeMirror actualType; + /** + * The default annotation for this slot from the real type factory. + * This field is nullable because we find the default annotation only + * if {@link InferenceOptions#makeDefaultsExplicit} is true. + */ + protected final @Nullable AnnotationMirror defaultAnnotation; + /** * Should this slot be inserted back into the source code. * This should be false for types that have an implicit annotation @@ -18,14 +29,23 @@ public class SourceVariableSlot extends VariableSlot { private boolean insertable; /** - * @param location Used to locate this variable in code, see @AnnotationLocation - * @param id Unique identifier for this variable + * @param location used to locate this variable in code, see @AnnotationLocation + * @param id unique identifier for this variable * @param type the underlying type + * @param defaultAnnotation the default annotation (solution) for this slot, which can be null when + * {@link InferenceOptions#makeDefaultsExplicit} returns true * @param insertable indicates whether this slot should be inserted back into the source code */ - public SourceVariableSlot(int id, AnnotationLocation location, TypeMirror type, boolean insertable) { + public SourceVariableSlot( + int id, + AnnotationLocation location, + TypeMirror type, + @Nullable AnnotationMirror defaultAnnotation, + boolean insertable + ) { super(id, location); this.actualType = type; + this.defaultAnnotation = defaultAnnotation; this.insertable = insertable; } @@ -67,4 +87,8 @@ public boolean isInsertable() { public void setInsertable(boolean insertable) { this.insertable = insertable; } + + public @Nullable AnnotationMirror getDefaultAnnotation() { + return defaultAnnotation; + } } diff --git a/src/checkers/inference/model/serialization/JsonDeserializer.java b/src/checkers/inference/model/serialization/JsonDeserializer.java index 7e4bcce9..77aca6d7 100644 --- a/src/checkers/inference/model/serialization/JsonDeserializer.java +++ b/src/checkers/inference/model/serialization/JsonDeserializer.java @@ -208,7 +208,9 @@ public Map getAnnotationValues() { private Slot parseSlot(String slot) { if (slot.startsWith(VAR_PREFIX)) { int id = Integer.valueOf(slot.split(":")[1]); - return new SourceVariableSlot(id, AnnotationLocation.MISSING_LOCATION, null, true); + // TODO: Here we are creating a SourceVariableSlot without any detailed information. + // We should consider refactor this implementation. + return new SourceVariableSlot(id, AnnotationLocation.MISSING_LOCATION, null, null, true); } else { // TODO: THIS NEEDS FIXING AnnotationMirror value = annotationSerializer.deserialize(slot); diff --git a/src/checkers/inference/util/SlotDefaultTypeResolver.java b/src/checkers/inference/util/SlotDefaultTypeResolver.java new file mode 100644 index 00000000..cc3367c2 --- /dev/null +++ b/src/checkers/inference/util/SlotDefaultTypeResolver.java @@ -0,0 +1,187 @@ +package checkers.inference.util; + +import com.sun.source.tree.AnnotatedTypeTree; +import com.sun.source.tree.ArrayTypeTree; +import com.sun.source.tree.ClassTree; +import com.sun.source.tree.CompilationUnitTree; +import com.sun.source.tree.ParameterizedTypeTree; +import com.sun.source.tree.PrimitiveTypeTree; +import com.sun.source.tree.Tree; +import com.sun.source.tree.TypeParameterTree; +import com.sun.source.tree.WildcardTree; +import com.sun.source.util.TreeScanner; +import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.javacutil.TreeUtils; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + * A utility class to help SlotManager to find the real default type of + * each slot. + * + * Slot manager tries to create a slot for each type declaration or type + * use in the class, so the associated tree is usually a type tree. + * Currently, AnnotatedTypeFactory cannot properly determine the annotated + * type of the type tree because it's unaware of the tree's location. + * + * For example, AnnotatedTypeFactory returns the same annotated type for + * "Object" in the following two cases: + * 1. Object s = "123"; + * 2. List l = new ArrayList<>(); + * But it's possible to have a different default type for the upperbound. + * + * This class aims to properly find the real default type for type trees. + */ +public class SlotDefaultTypeResolver { + + public static Map resolve( + CompilationUnitTree root, + BaseAnnotatedTypeFactory realTypeFactory + ) { + DefaultTypeFinder finder = new DefaultTypeFinder(realTypeFactory); + finder.scan(root, null); + + return finder.defaultTypes; + } + + /** + * A tree visitor that focuses on collecting the real default types for + * type trees under this tree. + * + * The results may contain information for trees that are not a type + * tree. For example, the implements clause of a class can either be + * an IdentifierTree or a ParameterizedTypeTree, but we always store + * its type for simplicity. + */ + private static class DefaultTypeFinder extends TreeScanner { + + private final BaseAnnotatedTypeFactory realTypeFactory; + + // A mapping from a tree to its real default type. + private final Map defaultTypes; + + private DefaultTypeFinder(BaseAnnotatedTypeFactory realTypeFactory) { + this.realTypeFactory = realTypeFactory; + this.defaultTypes = new HashMap<>(); + } + + // Each visit method should call this method to get the default + // type of its argument. This ensures the correct type information + // is propagated downwards, especially for nested type trees. + private AnnotatedTypeMirror getDefaultTypeFor(Tree tree) { + // use a cached type when possible + AnnotatedTypeMirror defaultType = defaultTypes.get(tree); + if (defaultType == null) { + if (TreeUtils.isTypeTree(tree)) { + defaultType = realTypeFactory.getAnnotatedTypeFromTypeTree(tree); + } else { + defaultType = realTypeFactory.getAnnotatedType(tree); + } + + defaultTypes.put(tree, defaultType); + } + + return defaultType; + } + + @Override + public Void visitClass(ClassTree tree, Void unused) { + Tree ext = tree.getExtendsClause(); + if (ext != null) { + defaultTypes.put(ext, realTypeFactory.getTypeOfExtendsImplements(ext)); + } + + List impls = tree.getImplementsClause(); + for (Tree im : impls) { + defaultTypes.put(im, realTypeFactory.getTypeOfExtendsImplements(im)); + } + + return super.visitClass(tree, unused); + } + + @Override + public Void visitPrimitiveType(PrimitiveTypeTree tree, Void unused) { + getDefaultTypeFor(tree); + return super.visitPrimitiveType(tree, unused); + } + + @Override + public Void visitArrayType(ArrayTypeTree tree, Void unused) { + AnnotatedTypeMirror.AnnotatedArrayType defaultType = + (AnnotatedTypeMirror.AnnotatedArrayType) getDefaultTypeFor(tree); + + defaultTypes.put(tree.getType(), defaultType.getComponentType()); + + return super.visitArrayType(tree, unused); + } + + @Override + public Void visitParameterizedType(ParameterizedTypeTree tree, Void unused) { + AnnotatedTypeMirror.AnnotatedDeclaredType defaultType = + (AnnotatedTypeMirror.AnnotatedDeclaredType) getDefaultTypeFor(tree); + + List typeArgumentTrees = tree.getTypeArguments(); + List typeArgumentTypes = defaultType.getTypeArguments(); + + /* + Note that it's not guaranteed that typeArgumentTrees.size() == typeArgumentTypes.size(). + For example: + List strs = new ArrayList<>() + + In the tree of `ArrayList<>`, we have no type arguments, but its type does have "String" + as its type argument. + */ + assert typeArgumentTrees.size() == 0 || typeArgumentTrees.size() == typeArgumentTypes.size(); + for (int i = 0; i < typeArgumentTrees.size(); ++i) { + defaultTypes.put(typeArgumentTrees.get(i), typeArgumentTypes.get(i)); + } + + // Sometimes, the slot manager annotates the underlying type tree instead of this ParameterizedTypeTree + defaultTypes.put(tree.getType(), defaultType.getErased()); + + return super.visitParameterizedType(tree, unused); + } + + @Override + public Void visitWildcard(WildcardTree tree, Void unused) { + AnnotatedTypeMirror.AnnotatedWildcardType defaultType = + (AnnotatedTypeMirror.AnnotatedWildcardType) getDefaultTypeFor(tree); + + if (tree.getKind() == Tree.Kind.EXTENDS_WILDCARD) { + defaultTypes.put(tree.getBound(), defaultType.getExtendsBound()); + } else if (tree.getKind() == Tree.Kind.SUPER_WILDCARD) { + defaultTypes.put(tree.getBound(), defaultType.getSuperBound()); + } + + return super.visitWildcard(tree, unused); + } + + @Override + public Void visitTypeParameter(TypeParameterTree tree, Void unused) { + AnnotatedTypeMirror.AnnotatedTypeVariable defaultType = + (AnnotatedTypeMirror.AnnotatedTypeVariable) getDefaultTypeFor(tree); + + // Annotated types for TypeParameterTree have the following format: + // @LowerBound T extends @UpperBound ... + // So the type of this tree should really mean the lower bound. + defaultTypes.put(tree, defaultType.getLowerBound()); + for (Tree bound : tree.getBounds()) { + defaultTypes.put(bound, defaultType.getUpperBound()); + } + + return super.visitTypeParameter(tree, unused); + } + + @Override + public Void visitAnnotatedType(AnnotatedTypeTree tree, Void unused) { + AnnotatedTypeMirror defaultType = getDefaultTypeFor(tree); + + defaultTypes.put(tree.getUnderlyingType(), defaultType); + + return super.visitAnnotatedType(tree, unused); + } + } +} From 2debee118ea3728936636e91e7ddbeb15fc412a5 Mon Sep 17 00:00:00 2001 From: zcai Date: Thu, 10 Feb 2022 14:08:05 -0500 Subject: [PATCH 2/4] Update qualifier hierarchy of sparta checker (#380) --- .../SimpleFlowAnnotatedTypeFactory.java | 92 ++++++++++++++----- 1 file changed, 67 insertions(+), 25 deletions(-) diff --git a/src/sparta/checkers/SimpleFlowAnnotatedTypeFactory.java b/src/sparta/checkers/SimpleFlowAnnotatedTypeFactory.java index 99402652..632346e9 100644 --- a/src/sparta/checkers/SimpleFlowAnnotatedTypeFactory.java +++ b/src/sparta/checkers/SimpleFlowAnnotatedTypeFactory.java @@ -1,12 +1,13 @@ package sparta.checkers; import checkers.inference.BaseInferenceRealTypeFactory; -import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory; +import org.checkerframework.checker.nullness.qual.Nullable; import org.checkerframework.common.basetype.BaseTypeChecker; import org.checkerframework.framework.qual.LiteralKind; import org.checkerframework.framework.qual.TypeUseLocation; import org.checkerframework.framework.source.SourceChecker; import org.checkerframework.framework.type.AnnotatedTypeMirror; +import org.checkerframework.framework.type.ElementQualifierHierarchy; import org.checkerframework.framework.type.QualifierHierarchy; import org.checkerframework.framework.type.treeannotator.ListTreeAnnotator; import org.checkerframework.framework.type.treeannotator.LiteralTreeAnnotator; @@ -19,6 +20,7 @@ import org.checkerframework.javacutil.TreeUtils; import java.lang.annotation.Annotation; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -31,6 +33,7 @@ import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.PackageElement; import javax.lang.model.type.TypeKind; +import javax.lang.model.util.Elements; import sparta.checkers.iflow.util.IFlowUtils; import sparta.checkers.iflow.util.PFPermission; @@ -285,41 +288,30 @@ private void handlePolyFlow(Element element, AnnotatedTypeMirror type) { } } - @SuppressWarnings("deprecation") @Override protected QualifierHierarchy createQualifierHierarchy() { - return org.checkerframework.framework.util.MultiGraphQualifierHierarchy - .createMultiGraphQualifierHierarchy(this); + return new FlowQualifierHierarchy(this.getSupportedTypeQualifiers(), elements); } - @SuppressWarnings("deprecation") - @Override - public QualifierHierarchy createQualifierHierarchyWithMultiGraphFactory( - org.checkerframework.framework.util.MultiGraphQualifierHierarchy.MultiGraphFactory factory - ) { - return new FlowQualifierHierarchy(factory); - } - - // TODO(Zhiping): deprecate usage of MultiGraphQualifierHierarchy - @SuppressWarnings("deprecation") - protected class FlowQualifierHierarchy extends org.checkerframework.framework.util.MultiGraphQualifierHierarchy { + protected class FlowQualifierHierarchy extends ElementQualifierHierarchy { - public FlowQualifierHierarchy(MultiGraphFactory f) { - super(f); - polyQualifiers.clear(); - polyQualifiers.put(NOSINK, POLYSINK); - polyQualifiers.put(ANYSOURCE, POLYSOURCE); + public FlowQualifierHierarchy( + Collection> qualifierClasses, + Elements elements + ) { + super(qualifierClasses, elements); } - @Override public Set getTopAnnotations() { + @Override + public Set getTopAnnotations() { return Collections.singleton(checker instanceof IFlowSinkChecker ? - NOSINK : - ANYSOURCE); + NOSINK : + ANYSOURCE); } @Override public AnnotationMirror getTopAnnotation(AnnotationMirror start) { - if (start.toString().contains("Sink")) { + if (IFlowUtils.isSink(start)) { return NOSINK; } else { return ANYSOURCE; @@ -335,13 +327,59 @@ public Set getBottomAnnotations() { @Override public AnnotationMirror getBottomAnnotation(AnnotationMirror start) { - if (start.toString().contains("Sink")) { + if (IFlowUtils.isSink(start)) { return ANYSINK; } else { return NOSOURCE; } } + @Override + public @Nullable AnnotationMirror leastUpperBound(AnnotationMirror a1, AnnotationMirror a2) { + if (!AnnotationUtils.areSameByName(getTopAnnotation(a1), getTopAnnotation(a2))) { + return null; + } else if (isSubtype(a1, a2)) { + return a2; + } else if (isSubtype(a2, a1)) { + return a1; + } else if (isSourceQualifier(a1)) { + // Since the two annotations are same by name, they are both source qualifier. + Set lubPermissions = getSources(a1); + lubPermissions.addAll(getSources(a2)); + return buildAnnotationMirrorFlowPermission(Source.class, toPermissionArray(lubPermissions)); + } else { + // Since the two annotations are same by name, they are both sink qualifier. + assert isSinkQualifier(a1); + + Set lubPermissions = getSinks(a1); + lubPermissions.retainAll(getSinks(a2)); + return buildAnnotationMirrorFlowPermission(Sink.class, toPermissionArray(lubPermissions)); + } + } + + @Override + public @Nullable AnnotationMirror greatestLowerBound(AnnotationMirror a1, AnnotationMirror a2) { + if (!AnnotationUtils.areSameByName(getTopAnnotation(a1), getTopAnnotation(a2))) { + return null; + } else if (isSubtype(a1, a2)) { + return a1; + } else if (isSubtype(a2, a1)) { + return a2; + } else if (isSourceQualifier(a1)) { + // Since the two annotations are same by name, they are both source qualifier. + Set glbPermissions = getSources(a1); + glbPermissions.retainAll(getSources(a2)); + return buildAnnotationMirrorFlowPermission(Source.class, toPermissionArray(glbPermissions)); + } else { + // Since the two annotations are same by name, they are both sink qualifier. + assert isSinkQualifier(a1); + + Set glbPermissions = getSinks(a1); + glbPermissions.addAll(getSinks(a2)); + return buildAnnotationMirrorFlowPermission(Sink.class, toPermissionArray(glbPermissions)); + } + } + @Override public boolean isSubtype(AnnotationMirror subtype, AnnotationMirror supertype) { if (isPolySourceQualifier(supertype) && isPolySourceQualifier(subtype)) { @@ -376,6 +414,10 @@ public boolean isSubtype(AnnotationMirror subtype, AnnotationMirror supertype) { } } + private String[] toPermissionArray(Collection permissions) { + return permissions.stream().map(PFPermission::toString).toArray(String[]::new); + } + private boolean isSuperSet(Set superset, Set subset) { if (superset.containsAll(subset) || superset.contains(PFPermission.ANY)) { return true; From a61365e9fad68fef4fccb10363972b810753df59 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 15 Feb 2022 19:41:27 -0500 Subject: [PATCH 3/4] Bump gson from 2.8.9 to 2.9.0 (#386) --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index e59b451a..1e120f65 100644 --- a/build.gradle +++ b/build.gradle @@ -63,7 +63,7 @@ dependencies { // Serialize constraints implementation 'com.googlecode.json-simple:json-simple:1.1.1' // Pretty print serialized constraints - implementation 'com.google.code.gson:gson:2.8.9' + implementation 'com.google.code.gson:gson:2.9.0' implementation 'org.ow2.sat4j:org.ow2.sat4j.core:2.3.6' implementation 'org.ow2.sat4j:org.ow2.sat4j.maxsat:2.3.6' From 9e7fbeaf02335f75621b2470dd5288647da52499 Mon Sep 17 00:00:00 2001 From: Piyush Jha Date: Wed, 16 Feb 2022 06:32:58 +0530 Subject: [PATCH 4/4] Remove unnecessary handling of explicit extends/implements clauses (#379) --- src/checkers/inference/VariableAnnotator.java | 24 ++----------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/src/checkers/inference/VariableAnnotator.java b/src/checkers/inference/VariableAnnotator.java index 473b4eb7..fa6d073e 100644 --- a/src/checkers/inference/VariableAnnotator.java +++ b/src/checkers/inference/VariableAnnotator.java @@ -765,7 +765,8 @@ private boolean handleWasRawDeclaredTypes(AnnotatedDeclaredType adt) { } /** - * Visit the extends, implements, and type parameters of the given class type and tree. + * Handle implicit extends clauses and type parameters of the given class type and tree. + * Explicit extends and implements clauses are handled by {@link checkers.inference.InferenceAnnotatedTypeFactory#getTypeOfExtendsImplements} */ private void handleClassDeclaration(AnnotatedDeclaredType classType, ClassTree classTree) { final Tree extendsTree = classTree.getExtendsClause(); @@ -788,29 +789,8 @@ private void handleClassDeclaration(AnnotatedDeclaredType classType, ClassTree c List superTypes = classType.directSupertypes(); superTypes.get(0).replaceAnnotation(slotManager.getAnnotation(extendsSlot)); - } else { - // Since only extends trees with a non-null tree path are handled (see - // checkers.inference.InferenceTreeAnnotator#visitIdentifier for more details), - // here don't dig deeper onto the extends tree when the classTree path is null. - // Note: the classTree path is null when the variableAnnotater is visiting it from - // a different compilation unit. The extends tree should be annotated when the - // compiler moves forward to the compilation unit containing the class definition. - if (inferenceTypeFactory.getPath(classTree) != null) { - final AnnotatedTypeMirror extendsType = inferenceTypeFactory.getAnnotatedTypeFromTypeTree(extendsTree); - visit(extendsType, extendsTree); - } } -// // TODO: NOT SURE THIS HANDLES MEMBER SELECT CORRECTLY -// int interfaceIndex = 1; -// for(Tree implementsTree : classTree.getImplementsClause()) { -// final AnnotatedTypeMirror implementsType = inferenceTypeFactory.getAnnotatedTypeFromTypeTree(implementsTree); -// AnnotatedTypeMirror supertype = classType.directSuperTypes().get(interfaceIndex); -// assert supertype.getUnderlyingType() == implementsType.getUnderlyingType(); -// visit(supertype, implementsTree); -// interfaceIndex++; -// } -// if (InferenceMain.isHackMode( (classType.getTypeArguments().size() != classTree.getTypeParameters().size()))) { return;