Ensure that resources are kept if there are keep rules on R class fields
We would previously not keep the entries if the R class values was
embedded as static field values (no clinit)
The test is rewriting the generated R class to ensure that the field
has a static value instead of being initialized in clinit.
Note that this can only trigger if there are no references to the R class at all.
Change-Id: Id7d52c7e8af3b2463e6dc48d420b8da9fd3f2a83
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerAnalysisCollection.java b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerAnalysisCollection.java
index a54fd90..e4f0706 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerAnalysisCollection.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/EnqueuerAnalysisCollection.java
@@ -22,6 +22,7 @@
import com.android.tools.r8.shaking.Enqueuer;
import com.android.tools.r8.shaking.Enqueuer.FieldAccessKind;
import com.android.tools.r8.shaking.EnqueuerWorklist;
+import com.android.tools.r8.shaking.KeepReason;
import com.android.tools.r8.utils.ArrayUtils;
import com.android.tools.r8.utils.timing.Timing;
import java.util.ArrayList;
@@ -54,6 +55,7 @@
private final NewlyReachableFieldEnqueuerAnalysis[] newlyReachableFieldAnalyses;
private final NewlyReferencedFieldEnqueuerAnalysis[] newlyReferencedFieldAnalyses;
private final NewlyTargetedMethodEnqueuerAnalysis[] newlyTargetedMethodAnalyses;
+ private final MarkFieldAsKeptEnqueuerAnalysis[] markFieldAsKeptEnqueuerAnalyses;
// Tear down events.
private final FinishedEnqueuerAnalysis[] finishedAnalyses;
@@ -81,6 +83,7 @@
NewlyReachableFieldEnqueuerAnalysis[] newlyReachableFieldAnalyses,
NewlyReferencedFieldEnqueuerAnalysis[] newlyReferencedFieldAnalyses,
NewlyTargetedMethodEnqueuerAnalysis[] newlyTargetedMethodAnalyses,
+ MarkFieldAsKeptEnqueuerAnalysis[] markFieldAsKeptEnqueuerAnalyses,
// Tear down events.
FinishedEnqueuerAnalysis[] finishedAnalyses,
FixpointEnqueuerAnalysis[] fixpointAnalyses) {
@@ -105,6 +108,7 @@
this.newlyReachableFieldAnalyses = newlyReachableFieldAnalyses;
this.newlyReferencedFieldAnalyses = newlyReferencedFieldAnalyses;
this.newlyTargetedMethodAnalyses = newlyTargetedMethodAnalyses;
+ this.markFieldAsKeptEnqueuerAnalyses = markFieldAsKeptEnqueuerAnalyses;
// Tear down events.
this.finishedAnalyses = finishedAnalyses;
this.fixpointAnalyses = fixpointAnalyses;
@@ -346,6 +350,13 @@
}
}
+ public void processMarkFieldKept(
+ ProgramField field, KeepReason reason, EnqueuerWorklist worklist) {
+ for (MarkFieldAsKeptEnqueuerAnalysis analysis : markFieldAsKeptEnqueuerAnalyses) {
+ analysis.processNewlyKeptField(field, reason, worklist);
+ }
+ }
+
// Tear down events.
public void done(Enqueuer enqueuer) {
@@ -406,6 +417,7 @@
new ArrayList<>();
private final List<NewlyTargetedMethodEnqueuerAnalysis> newlyTargetedMethodAnalyses =
new ArrayList<>();
+ private final List<MarkFieldAsKeptEnqueuerAnalysis> markFieldAsKeptAnalyses = new ArrayList<>();
// Tear down events.
private final List<FinishedEnqueuerAnalysis> finishedAnalyses = new ArrayList<>();
@@ -511,6 +523,11 @@
return this;
}
+ public Builder addMarkFieldAsKeptAnalysis(MarkFieldAsKeptEnqueuerAnalysis analysis) {
+ markFieldAsKeptAnalyses.add(analysis);
+ return this;
+ }
+
// Tear down events.
public Builder addFinishedAnalysis(FinishedEnqueuerAnalysis analysis) {
@@ -547,6 +564,7 @@
newlyReachableFieldAnalyses.toArray(NewlyReachableFieldEnqueuerAnalysis[]::new),
newlyReferencedFieldAnalyses.toArray(NewlyReferencedFieldEnqueuerAnalysis[]::new),
newlyTargetedMethodAnalyses.toArray(NewlyTargetedMethodEnqueuerAnalysis[]::new),
+ markFieldAsKeptAnalyses.toArray(MarkFieldAsKeptEnqueuerAnalysis[]::new),
// Tear down events.
finishedAnalyses.toArray(FinishedEnqueuerAnalysis[]::new),
fixpointAnalyses.toArray(FixpointEnqueuerAnalysis[]::new));
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/MarkFieldAsKeptEnqueuerAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/MarkFieldAsKeptEnqueuerAnalysis.java
new file mode 100644
index 0000000..efec9bc
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/analysis/MarkFieldAsKeptEnqueuerAnalysis.java
@@ -0,0 +1,14 @@
+// Copyright (c) 2025, 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.graph.analysis;
+
+import com.android.tools.r8.graph.ProgramField;
+import com.android.tools.r8.shaking.EnqueuerWorklist;
+import com.android.tools.r8.shaking.KeepReason;
+
+public interface MarkFieldAsKeptEnqueuerAnalysis {
+
+ /** Called when a field is marked as kept. */
+ void processNewlyKeptField(ProgramField field, KeepReason keepReason, EnqueuerWorklist worklist);
+}
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
index 660504d..b0376cc 100644
--- a/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
+++ b/src/main/java/com/android/tools/r8/graph/analysis/ResourceAccessAnalysis.java
@@ -27,12 +27,14 @@
import com.android.tools.r8.ir.conversion.MethodConversionOptions;
import com.android.tools.r8.shaking.Enqueuer;
import com.android.tools.r8.shaking.EnqueuerWorklist;
+import com.android.tools.r8.shaking.KeepReason;
import it.unimi.dsi.fastutil.ints.IntArrayList;
import it.unimi.dsi.fastutil.ints.IntList;
import java.util.IdentityHashMap;
import java.util.Map;
-public class ResourceAccessAnalysis implements TraceFieldAccessEnqueuerAnalysis {
+public class ResourceAccessAnalysis
+ implements TraceFieldAccessEnqueuerAnalysis, MarkFieldAsKeptEnqueuerAnalysis {
private final R8ResourceShrinkerState resourceShrinkerState;
private final Map<DexType, RClassFieldToValueStore> fieldToValueMapping = new IdentityHashMap<>();
@@ -51,7 +53,9 @@
Enqueuer enqueuer,
EnqueuerAnalysisCollection.Builder builder) {
if (fieldAccessAnalysisEnabled(appView, enqueuer)) {
- builder.addTraceFieldAccessAnalysis(new ResourceAccessAnalysis(appView, enqueuer));
+ ResourceAccessAnalysis analysis = new ResourceAccessAnalysis(appView, enqueuer);
+ builder.addTraceFieldAccessAnalysis(analysis);
+ builder.addMarkFieldAsKeptAnalysis(analysis);
}
if (liveFieldAnalysisEnabled(appView, enqueuer)) {
builder.addNewlyLiveFieldAnalysis(
@@ -93,12 +97,21 @@
}
@Override
+ public void processNewlyKeptField(
+ ProgramField field, KeepReason keepReason, EnqueuerWorklist worklist) {
+ processField(field);
+ }
+
+ @Override
public void traceStaticFieldRead(
DexField field,
SingleFieldResolutionResult<?> resolutionResult,
ProgramMethod context,
EnqueuerWorklist worklist) {
- ProgramField resolvedField = resolutionResult.getProgramField();
+ processField(resolutionResult.getProgramField());
+ }
+
+ private void processField(ProgramField resolvedField) {
if (resolvedField == null) {
return;
}
@@ -109,12 +122,12 @@
}
assert fieldToValueMapping.containsKey(holderType);
RClassFieldToValueStore rClassFieldToValueStore = fieldToValueMapping.get(holderType);
- IntList integers = rClassFieldToValueStore.valueMapping.get(field);
+ IntList integers = rClassFieldToValueStore.valueMapping.get(resolvedField.getReference());
// The R class can have fields injected, e.g., by jacoco, we don't have resource values for
// these.
if (integers != null) {
for (Integer integer : integers) {
- resourceShrinkerState.trace(integer, field.toString());
+ resourceShrinkerState.trace(integer, resolvedField.getReference().toString());
}
}
}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 3021e9c..15a2384 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -4999,6 +4999,7 @@
} else {
worklist.enqueueMarkFieldAsReachableAction(field, field, reason);
}
+ analyses.processMarkFieldKept(field, reason, worklist);
}
private boolean shouldMarkLibraryMethodOverrideAsReachable(LookupTarget override) {
diff --git a/src/test/java/com/android/tools/r8/androidresources/KeptOnlyRClassEntriesTest.java b/src/test/java/com/android/tools/r8/androidresources/KeptOnlyRClassEntriesTest.java
new file mode 100644
index 0000000..42c61c6
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/KeptOnlyRClassEntriesTest.java
@@ -0,0 +1,98 @@
+// Copyright (c) 2025, 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.androidresources;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.TestResourceTable;
+import com.android.tools.r8.transformers.ClassTransformer;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.MethodVisitor;
+
+@RunWith(Parameterized.class)
+public class KeptOnlyRClassEntriesTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withDefaultDexRuntime().withAllApiLevels().build();
+ }
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp) throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.drawable.class)
+ .build(temp);
+ }
+
+ @Test
+ public void testKeptField() throws Exception {
+ AndroidTestResource testResources = getTestResources(temp);
+ TestResourceTable testResourceTable = testResources.getTestResourceTable();
+ testResources
+ .getRClass()
+ .transformClassFileData(
+ new ClassTransformer() {
+ @Override
+ public FieldVisitor visitField(
+ int access, String name, String descriptor, String signature, Object value) {
+ if (name.equals("kept_field")) {
+ value = Integer.valueOf(testResourceTable.idFor("drawable", "kept_field"));
+ }
+ return super.visitField(access, name, descriptor, signature, value);
+ }
+
+ @Override
+ public MethodVisitor visitMethod(
+ int access,
+ String name,
+ String descriptor,
+ String signature,
+ String[] exceptions) {
+ if (name.equals("<clinit>")) {
+ // We are explicitly setting the value in the field visitor above
+ return null;
+ }
+ return super.visitMethod(access, name, descriptor, signature, exceptions);
+ }
+ },
+ s -> s.contains("drawable"));
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(TestClass.class)
+ .addAndroidResources(testResources)
+ .addKeepMainRule(TestClass.class)
+ .addKeepRules("-keep class **R*drawable { int kept_field;}")
+ .enableOptimizedShrinking()
+ .compile()
+ .inspectShrunkenResources(
+ resourceTableInspector -> {
+ resourceTableInspector.assertContainsResourceWithName("drawable", "kept_field");
+ })
+ .run(parameters.getRuntime(), TestClass.class)
+ .assertSuccess();
+ }
+
+ public static class TestClass {
+ public static void main(String[] args) {}
+ }
+
+ public static class R {
+
+ public static class drawable {
+ public static int kept_field = 0x7f010001;
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/androidresources/RClassResourceGeneration.java b/src/test/java/com/android/tools/r8/androidresources/RClassResourceGeneration.java
index 9f5f557..4d2a0e8 100644
--- a/src/test/java/com/android/tools/r8/androidresources/RClassResourceGeneration.java
+++ b/src/test/java/com/android/tools/r8/androidresources/RClassResourceGeneration.java
@@ -71,7 +71,7 @@
public void testResourceRewriting() throws Exception {
AndroidApp resourceApp =
AndroidApp.builder()
- .addClassProgramData(testResource.getRClass().getClassFileData())
+ .addClassProgramData(testResource.getRClass().getClassFileData().values())
.build();
CodeInspector inspector = new CodeInspector(resourceApp);
ClassSubject stringClazz = inspector.clazz(R.string.class);
diff --git a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
index 3d46d18..edd0bbf 100644
--- a/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
+++ b/src/test/testbase/java/com/android/tools/r8/R8TestBuilder.java
@@ -1043,16 +1043,16 @@
return featureSplitGenerator.build();
});
- return addProgramClassFileData(testResource.getRClass().getClassFileData());
+ return addProgramClassFileData(testResource.getRClass().getClassFileData().values());
}
public T addAndroidResources(AndroidTestResource testResource, Path output) throws IOException {
- List<byte[]> classFileData = testResource.getRClass().getClassFileData();
+ Collection<byte[]> classFileData = testResource.getRClass().getClassFileData().values();
return addAndroidResources(testResource, output, classFileData);
}
public T addAndroidResources(
- AndroidTestResource testResource, Path output, List<byte[]> classFileData) {
+ AndroidTestResource testResource, Path output, Collection<byte[]> classFileData) {
Path resources = testResource.getResourceZip();
resourceShrinkerOutput = output;
ArchiveProtoAndroidResourceProvider provider = getResourceProvider(testResource);
diff --git a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
index d55db2f..f2e8b29 100644
--- a/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
+++ b/src/test/testbase/java/com/android/tools/r8/androidresources/AndroidResourceTestingUtils.java
@@ -9,13 +9,16 @@
import com.android.aapt.Resources;
import com.android.aapt.Resources.ConfigValue;
import com.android.aapt.Resources.Item;
+import com.android.aapt.Resources.Package;
import com.android.aapt.Resources.ResourceTable;
+import com.android.build.shrinker.ResourceTableUtilKt;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestBase.Backend;
import com.android.tools.r8.TestRuntime.CfRuntime;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ProcessResult;
import com.android.tools.r8.graph.DexItemFactory;
+import com.android.tools.r8.transformers.ClassFileTransformer;
import com.android.tools.r8.transformers.ClassTransformer;
import com.android.tools.r8.transformers.MethodTransformer;
import com.android.tools.r8.utils.AndroidApiLevel;
@@ -24,6 +27,7 @@
import com.android.tools.r8.utils.StreamUtils;
import com.android.tools.r8.utils.StringUtils;
import com.android.tools.r8.utils.ZipUtils;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.MoreCollectors;
import com.google.protobuf.InvalidProtocolBufferException;
@@ -36,6 +40,7 @@
import java.nio.file.StandardOpenOption;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -44,6 +49,7 @@
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.function.Function;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
@@ -160,9 +166,9 @@
private final Path javaFilePath;
// The compiled class files, with the class names rewritten to the names used in passed
// in R class from the test.
- private final List<byte[]> classFileData;
+ private final Map<String, byte[]> classFileData;
- AndroidTestRClass(Path javaFilePath, List<byte[]> classFileData) throws IOException {
+ AndroidTestRClass(Path javaFilePath, Map<String, byte[]> classFileData) throws IOException {
this.javaFilePath = javaFilePath;
this.classFileData = classFileData;
}
@@ -170,9 +176,26 @@
return javaFilePath;
}
- public List<byte[]> getClassFileData() {
+ public Map<String, byte[]> getClassFileData() {
return classFileData;
}
+
+ public void transformClassFileData(ClassTransformer transformer) {
+ transformClassFileData(transformer, s -> true);
+ }
+
+ public void transformClassFileData(ClassTransformer transformer, Predicate<String> include) {
+ Map<String, byte[]> changed = new HashMap<>();
+ for (Entry<String, byte[]> entry : classFileData.entrySet()) {
+ if (include.test(entry.getKey())) {
+ byte[] transform =
+ ClassFileTransformer.transform(
+ entry.getValue(), ImmutableList.of(transformer), Collections.EMPTY_LIST, 0);
+ changed.put(entry.getKey(), transform);
+ }
+ }
+ classFileData.putAll(changed);
+ }
}
public static class AndroidTestResource {
@@ -209,6 +232,11 @@
return this;
}
+ public TestResourceTable getTestResourceTable() throws IOException {
+ return new TestResourceTable(
+ ResourceTable.parseFrom(ZipUtils.readSingleEntry(resourceZip, "resources.pb")));
+ }
+
public AndroidTestRClass getRClass() {
return rClass;
}
@@ -224,14 +252,17 @@
// Easy traversable resource table.
public static class TestResourceTable {
- private Map<String, ResourceNameToValueMapping> mapping = new HashMap<>();
+ private Map<String, ResourceNameToValueMapping> valueMapping = new HashMap<>();
+ private Map<String, ResourceNameToIdMapping> idMapping = new HashMap<>();
private TestResourceTable(ResourceTable resourceTable) {
// For now, we don't have any test that use multiple packages.
assert resourceTable.getPackageCount() == 1;
- for (Resources.Type type : resourceTable.getPackage(0).getTypeList()) {
+ Package aPackage = resourceTable.getPackage(0);
+ for (Resources.Type type : aPackage.getTypeList()) {
String typeName = type.getName();
- mapping.put(typeName, new ResourceNameToValueMapping(type));
+ valueMapping.put(typeName, new ResourceNameToValueMapping(type));
+ idMapping.put(typeName, new ResourceNameToIdMapping(type, aPackage));
}
}
@@ -240,11 +271,29 @@
}
public boolean containsValueFor(String type, String name) {
- return mapping.containsKey(type) && mapping.get(type).containsValueFor(name);
+ return valueMapping.containsKey(type) && valueMapping.get(type).containsValueFor(name);
+ }
+
+ public int idFor(String type, String name) {
+ return idMapping.get(type).getIdFor(name);
}
public Collection<String> entriesForType(String type) {
- return mapping.get(type).mapping.keySet();
+ return valueMapping.get(type).mapping.keySet();
+ }
+
+ public static class ResourceNameToIdMapping {
+ private final Map<String, Integer> mapping = new HashMap<>();
+
+ public ResourceNameToIdMapping(Resources.Type type, Package aPackage) {
+ for (Resources.Entry entry : type.getEntryList()) {
+ mapping.put(entry.getName(), ResourceTableUtilKt.toIdentifier(aPackage, type, entry));
+ }
+ }
+
+ public int getIdFor(String name) {
+ return mapping.get(name);
+ }
}
public static class ResourceNameToValueMapping {
@@ -541,12 +590,13 @@
Collectors.toMap(
AndroidResourceTestingUtils::rClassWithoutNamespaceAndOuter,
DescriptorUtils::getClassBinaryName));
- List<byte[]> rewrittenRClassFiles = new ArrayList<>();
+ Map<String, byte[]> rewrittenRClassFiles = new HashMap<>();
ZipUtils.iter(
rClassClassFileOutput,
(entry, input) -> {
if (ZipUtils.isClassFile(entry.getName()) && !entry.getName().endsWith("R.class")) {
- rewrittenRClassFiles.add(
+ rewrittenRClassFiles.put(
+ entry.getName(),
transformer(StreamUtils.streamToByteArrayClose(input), null)
.addClassTransformer(
new ClassTransformer() {
