Test warnings for package private access across isolated split Bug: b/300247439 Change-Id: Ibe735397e38d45b9bbe44423cc50c56964707994
diff --git a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java index 0e79887..a20cfef 100644 --- a/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java +++ b/src/main/java/com/android/tools/r8/features/ClassToFeatureSplitMap.java
@@ -12,6 +12,7 @@ import com.android.tools.r8.graph.AppView; import com.android.tools.r8.graph.DexItemFactory; import com.android.tools.r8.graph.DexProgramClass; +import com.android.tools.r8.graph.DexReference; import com.android.tools.r8.graph.DexType; import com.android.tools.r8.graph.ProgramDefinition; import com.android.tools.r8.graph.PrunedItems; @@ -128,17 +129,16 @@ public FeatureSplit getFeatureSplit( ProgramDefinition definition, SyntheticItems syntheticItems) { - return getFeatureSplit(definition.getContextType(), syntheticItems); + return getFeatureSplit(definition.getReference(), syntheticItems); } public FeatureSplit getFeatureSplit( - DexType type, AppView<? extends AppInfoWithClassHierarchy> appView) { - return getFeatureSplit(type, appView.getSyntheticItems()); + DexReference reference, AppView<? extends AppInfoWithClassHierarchy> appView) { + return getFeatureSplit(reference, appView.getSyntheticItems()); } - public FeatureSplit getFeatureSplit( - DexType type, - SyntheticItems syntheticItems) { + public FeatureSplit getFeatureSplit(DexReference reference, SyntheticItems syntheticItems) { + DexType type = reference.getContextType(); if (syntheticItems == null) { // Called from AndroidApp.dumpProgramResources(). return classToFeatureSplitMap.getOrDefault(type, FeatureSplit.BASE); @@ -216,6 +216,16 @@ return !isInBase(clazz, syntheticItems); } + public boolean isInSameFeature( + ProgramDefinition definition, ProgramDefinition other, AppView<?> appView) { + return isInSameFeature(definition, other, appView.getSyntheticItems()); + } + + public boolean isInSameFeature( + ProgramDefinition definition, ProgramDefinition other, SyntheticItems syntheticItems) { + return getFeatureSplit(definition, syntheticItems) == getFeatureSplit(other, syntheticItems); + } + public ClassToFeatureSplitMap rewrittenWithLens(GraphLens lens, Timing timing) { return timing.time("Rewrite ClassToFeatureSplitMap", () -> rewrittenWithLens(lens)); }
diff --git a/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java b/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java index 5708818..dbef2b3 100644 --- a/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java +++ b/src/main/java/com/android/tools/r8/features/IsolatedFeatureSplitsChecker.java
@@ -79,7 +79,7 @@ } private void checkAccess(ProgramDefinition accessedItem, ProgramMethod context) { - if (!features.isInBaseOrSameFeatureAs(accessedItem, context, appView) + if (!features.isInSameFeature(accessedItem, context, appView) && !accessedItem.getAccessFlags().isPublic()) { appView .reporter()
diff --git a/src/test/java/com/android/tools/r8/R8TestBuilder.java b/src/test/java/com/android/tools/r8/R8TestBuilder.java index abafdc8..0e08d8f 100644 --- a/src/test/java/com/android/tools/r8/R8TestBuilder.java +++ b/src/test/java/com/android/tools/r8/R8TestBuilder.java
@@ -75,6 +75,7 @@ private AllowedDiagnosticMessages allowedDiagnosticMessages = AllowedDiagnosticMessages.NONE; private boolean allowUnusedProguardConfigurationRules = false; + private boolean enableIsolatedSplits = false; private boolean enableMissingLibraryApiModeling = true; private boolean enableStartupLayoutOptimization = true; private CollectingGraphConsumer graphConsumer = null; @@ -152,6 +153,7 @@ ToolHelper.addSyntheticProguardRulesConsumerForTesting( builder, rules -> box.syntheticProguardRules = rules); libraryDesugaringTestConfiguration.configure(builder); + builder.setEnableExperimentalIsolatedSplits(enableIsolatedSplits); builder.setEnableExperimentalMissingLibraryApiModeling(enableMissingLibraryApiModeling); builder.setEnableStartupLayoutOptimization(enableStartupLayoutOptimization); StringBuilder pgConfOutput = wrapProguardConfigConsumer(builder); @@ -856,6 +858,11 @@ return self(); } + public T enableIsolatedSplits(boolean enableIsolatedSplits) { + this.enableIsolatedSplits = enableIsolatedSplits; + return self(); + } + public T addArtProfileForRewriting(ArtProfileProvider artProfileProvider) { return addArtProfileForRewriting( artProfileProvider,
diff --git a/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java new file mode 100644 index 0000000..1b24d61 --- /dev/null +++ b/src/test/java/com/android/tools/r8/features/PackagePrivateIsolatedSplitCrossBoundaryReferenceTest.java
@@ -0,0 +1,151 @@ +// Copyright (c) 2023, the R8 project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. +package com.android.tools.r8.features; + +import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage; +import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType; +import static com.android.tools.r8.utils.codeinspector.AssertUtils.assertFailsCompilationIf; +import static org.hamcrest.CoreMatchers.allOf; +import static org.hamcrest.CoreMatchers.equalTo; + +import com.android.tools.r8.TestBase; +import com.android.tools.r8.TestCompilerBuilder.DiagnosticsConsumer; +import com.android.tools.r8.TestDiagnosticMessages; +import com.android.tools.r8.TestParameters; +import com.android.tools.r8.features.diagnostic.IllegalAccessWithIsolatedFeatureSplitsDiagnostic; +import com.android.tools.r8.references.ClassReference; +import com.android.tools.r8.references.MethodReference; +import com.android.tools.r8.references.Reference; +import com.android.tools.r8.utils.BooleanUtils; +import com.android.tools.r8.utils.MethodReferenceUtils; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class PackagePrivateIsolatedSplitCrossBoundaryReferenceTest extends TestBase { + + @Parameter(0) + public boolean enableIsolatedSplits; + + @Parameter(1) + public TestParameters parameters; + + @Parameters(name = "{1}, isolated splits: {0}") + public static List<Object[]> data() { + return buildParameters( + BooleanUtils.values(), getTestParameters().withDexRuntimesAndAllApiLevels().build()); + } + + @Test + public void testPublicToPublic() throws Exception { + runTest("testPublicToPublic", TestDiagnosticMessages::assertNoMessages); + } + + @Test + public void testPublicToNonPublic() throws Exception { + MethodReference accessedMethod = + Reference.methodFromMethod(NonPublicBase.class.getDeclaredMethod("nonPublicMethod")); + MethodReference context = + Reference.methodFromMethod(Feature.class.getDeclaredMethod("testPublicToNonPublic")); + assertFailsCompilationIf( + enableIsolatedSplits, + () -> + runTest( + "testPublicToNonPublic", + diagnostics -> + diagnostics.assertErrorsMatch( + allOf( + diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class), + diagnosticMessage( + equalTo(getExpectedDiagnosticMessage(accessedMethod, context))))))); + } + + @Test + public void testNonPublicToPublic() throws Exception { + ClassReference accessedClass = Reference.classFromClass(NonPublicBaseSub.class); + MethodReference context = + Reference.methodFromMethod(Feature.class.getDeclaredMethod("testNonPublicToPublic")); + assertFailsCompilationIf( + enableIsolatedSplits, + () -> + runTest( + "testNonPublicToPublic", + diagnostics -> + diagnostics.assertErrorsMatch( + allOf( + diagnosticType(IllegalAccessWithIsolatedFeatureSplitsDiagnostic.class), + diagnosticMessage( + equalTo(getExpectedDiagnosticMessage(accessedClass, context))))))); + } + + private static String getExpectedDiagnosticMessage( + ClassReference accessedClass, MethodReference context) { + return "Unexpected illegal access to non-public class in another feature split " + + "(accessed: " + + accessedClass.getTypeName() + + ", context: " + + MethodReferenceUtils.toSourceString(context) + + ")."; + } + + private static String getExpectedDiagnosticMessage( + MethodReference accessedMethod, MethodReference context) { + return "Unexpected illegal access to non-public method in another feature split " + + "(accessed: " + + MethodReferenceUtils.toSourceString(accessedMethod) + + ", context: " + + MethodReferenceUtils.toSourceString(context) + + ")."; + } + + private void runTest(String name, DiagnosticsConsumer inspector) throws Exception { + testForR8(parameters.getBackend()) + .addProgramClasses(NonPublicBase.class, PublicBaseSub.class, NonPublicBaseSub.class) + .addKeepClassAndMembersRules( + NonPublicBase.class, PublicBaseSub.class, NonPublicBaseSub.class) + .addFeatureSplit(Feature.class) + .addKeepRules("-keep class " + Feature.class.getTypeName() + " { void " + name + "(); }") + .enableIsolatedSplits(enableIsolatedSplits) + .setMinApi(parameters) + .compileWithExpectedDiagnostics(enableIsolatedSplits ? inspector : this::assertNoMessages); + } + + private void assertNoMessages(TestDiagnosticMessages diagnostics) { + diagnostics.assertNoMessages(); + } + + static class NonPublicBase { + + public static void publicMethod() { + // Intentionally empty. + } + + static void nonPublicMethod() { + // Intentionally empty. + } + } + + public static class PublicBaseSub extends NonPublicBase {} + + static class NonPublicBaseSub extends NonPublicBase {} + + public static class Feature { + + public static void testPublicToPublic() { + PublicBaseSub.publicMethod(); + } + + public static void testPublicToNonPublic() { + PublicBaseSub.nonPublicMethod(); + } + + public static void testNonPublicToPublic() { + NonPublicBaseSub.publicMethod(); + } + } +}