A follow-up investigation revealed by #1853:
The value stored in fakeOverrides for a stub fake override is a
getAnnotatedType(overridden) snapshot taken at parse/application time
(AnnotationFileParser.processFakeOverride; BinaryStubReader.applyFakeOverride). If the
overridden method is declared in a member of the same file/top-level class that has not
been processed yet, the snapshot bakes in that member's not-yet-applied state — and
fakeOverrides entries are never invalidated.
Both paths share the hazard, in the same order:
- The text parser processes declarations top-to-bottom within a stub file and snapshots
when it reaches the fake declaration.
- The binary reader applies the outer class record, then inner-class records in sequence
(AnnotationFileElementTypes.maybeParseEnclosingJdkClass); a fake override in inner
class A that targets a method declared in sibling inner class B applied later snapshots
B's incomplete state. (parsingCount bracketing, commit c0ba0bba4, prevents the
ATF caches from being poisoned mid-application, but the stored snapshot itself is
outside those caches.)
Because both sides agree on the (possibly order-dependent) value, the
BinaryStubDiffChecker harness reports 0 mismatches — equivalence holds, correctness of
the agreed value does not necessarily.
Concrete JDK shape where this matters: TreeMap.AscendingSubMap extends TreeMap.NavigableSubMap, and NavigableSubMap's fake overrides of the java.util.Map
default methods are load-bearing since commit bc1cbf69 (unannotated fake overrides now
reset to stub defaults on both paths).
Proposed fix: two-phase processing — apply all atypes/declAnnos for the whole stub
file (text) / class-record set (binary) first, then process fake overrides in a second
pass. Must be done on both paths in the same change, or the diff harness will
(correctly) report mismatches.
A follow-up investigation revealed by #1853:
The value stored in
fakeOverridesfor a stub fake override is agetAnnotatedType(overridden)snapshot taken at parse/application time(
AnnotationFileParser.processFakeOverride;BinaryStubReader.applyFakeOverride). If theoverridden method is declared in a member of the same file/top-level class that has not
been processed yet, the snapshot bakes in that member's not-yet-applied state — and
fakeOverridesentries are never invalidated.Both paths share the hazard, in the same order:
when it reaches the fake declaration.
(
AnnotationFileElementTypes.maybeParseEnclosingJdkClass); a fake override in innerclass A that targets a method declared in sibling inner class B applied later snapshots
B's incomplete state. (
parsingCountbracketing, commitc0ba0bba4, prevents theATF caches from being poisoned mid-application, but the stored snapshot itself is
outside those caches.)
Because both sides agree on the (possibly order-dependent) value, the
BinaryStubDiffCheckerharness reports 0 mismatches — equivalence holds, correctness ofthe agreed value does not necessarily.
Concrete JDK shape where this matters:
TreeMap.AscendingSubMap extends TreeMap.NavigableSubMap, andNavigableSubMap's fake overrides of thejava.util.Mapdefault methods are load-bearing since commit
bc1cbf69(unannotated fake overrides nowreset to stub defaults on both paths).
Proposed fix: two-phase processing — apply all
atypes/declAnnosfor the whole stubfile (text) / class-record set (binary) first, then process fake overrides in a second
pass. Must be done on both paths in the same change, or the diff harness will
(correctly) report mismatches.