Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,15 @@ public AnnotationMirror createUnderInitializationAnnotation(TypeMirror typeFrame
public @Nullable AnnotatedDeclaredType getSelfType(Tree tree) {
AnnotatedDeclaredType selfType = super.getSelfType(tree);

if (assumeInitialized) {
if (assumeInitialized || selfType == null) {
return selfType;
}

// The loop below mutates the self type and its enclosing types
// (setSelfTypeInInitializationCode), so copy first: super.getSelfType may return a shared,
// immutable type from a cache.
selfType = selfType.deepCopy();

TreePath path = getPath(tree);
AnnotatedDeclaredType enclosing = selfType;
while (path != null && enclosing != null) {
Expand Down
28 changes: 28 additions & 0 deletions checker/tests/nullness/ElementTypeCacheWildcardBound.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Regression test: a (since-reverted) elementTypeCache "boundary flip" returned the shared
// frozen cache master from getAnnotatedType(Element) instead of a deep copy. A consumer then
// embedded a *sub-component* of that shared value -- here an unbounded wildcard's implicit
// java.lang.Object upper bound, derived from a JDK generic type's cached type-parameter bound --
// into a fresh, non-frozen result type. Because the root was not frozen, the copy-on-frozen
// guards (which check only the root) did not copy it, and addComputedTypeAnnotations later
// mutated the frozen child, crashing with "Attempted to mutate a frozen AnnotatedTypeMirror with
// underlying type java.lang.Object".
//
// It needs a JDK parameterized type (whose annotated type is cached) used with an unbounded
// wildcard, plus a raw-generic-bounded type variable, in a generic method that is invoked so the
// dataflow transfer re-derives the local's type. Minimized from Guava's
// com.google.common.collect.SortedLists.binarySearch, which alltests did not cover. This test
// should type-check with no errors.

import java.util.function.Function;

public class ElementTypeCacheWildcardBound {
@SuppressWarnings("rawtypes")
static <K extends Comparable> int f(Function<?, K> fn) {
return g(fn);
}

@SuppressWarnings("rawtypes")
static <K extends Comparable> int g(Function<?, K> fn) {
return 0;
}
}
19 changes: 19 additions & 0 deletions checker/tests/nullness/VarargCacheAliasing.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test: AnnotatedTypeCopier.visitExecutable used to alias (rather than copy) the
// vararg type when deep-copying an executable type, so deepCopy() did not produce a fully
// independent type. The shared vararg subtree was then re-defaulted in place. This is benign
// while cached types are mutable, but corrupts a shared cached value once cache masters are
// frozen; with that enforcement it crashed with "Attempted to mutate a frozen
// AnnotatedTypeMirror". Calling JDK vararg methods (whose executable types are cached and
// re-defaulted) reproduces it. This test should type-check with no errors.

import java.util.Arrays;
import java.util.List;

public class VarargCacheAliasing {
void use(String s) throws Exception {
List<String> xs = Arrays.asList(s, s, s);
List<String> ys = Arrays.asList(s);
String f = String.format("%s %s", s, s);
java.lang.reflect.Method m = VarargCacheAliasing.class.getMethod("use", String.class);
}
}
25 changes: 25 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,31 @@ thread-local map instead of allocating one per copy. Total allocation on large s
dropped 14-19% (e.g. -19% on a 1500-method class) and about 7% on a many-file corpus; wall clock is
roughly unchanged, the gain being reduced GC pressure. No user-visible behavior change.

`AnnotatedTypeMirror` has new `freeze()` and `isFrozen()` methods. Freezing a type
makes it (and every type reachable from it) reject primary-annotation changes,
throwing `BugInCF` on an attempted mutation; lazy initialization of bounds and type
arguments is still permitted. This lets the framework share an immutable cached type
without defensively deep-copying it.

Fixed a latent aliasing bug in `AnnotatedTypeCopier`: when copying an executable
type, the copy shared the original's vararg type instead of copying it, so
`deepCopy()` did not produce a fully independent type. The annotated-type caches in
`AnnotatedTypeFactory` now freeze the value they store (the masters), so a future
in-place mutation of a cached type fails fast with `BugInCF` instead of silently
corrupting the shared value.

`StructuralEqualityComparer.arePrimaryAnnosEqual` is now non-mutating. The Value
Checker's override previously normalized the two operands' annotations to a canonical
form by calling `replaceAnnotation` on them -- an equality check with a side effect on
its operands, which also prevents comparing a shared immutable type. It now computes
the canonical annotations and compares them without mutating the types.

Performance: `getAnnotatedType(Tree)` for class and method declarations
(`classAndMethodTreeCache`) now returns the shared frozen cached type instead of a deep
copy on every hit; the few callers that mutate the result copy it first. This removes a
deep copy per cache hit for the read-only majority of callers. No user-visible behavior
change.

Fixed a bug that caused an IndexOutOfBoundsException for lambdas in varargs,
for type systems that had the Aliasing Checker as a subchecker, like the
Optional Checker.
Expand Down
Loading
Loading