Reland "Enforce class access flags to be valid."
Disabling the 004-JniTest, as it contains a smali file with invalid flags.
Bug:
Change-Id: I87c16697d14333a676d9f1dd6a72330732147fe6
diff --git a/src/main/java/com/android/tools/r8/dex/Constants.java b/src/main/java/com/android/tools/r8/dex/Constants.java
index 12ae77f..7a3d631 100644
--- a/src/main/java/com/android/tools/r8/dex/Constants.java
+++ b/src/main/java/com/android/tools/r8/dex/Constants.java
@@ -14,6 +14,9 @@
/** vdex file version number for Android O (API level 26) */
public static final int ANDROID_O_VDEX_VERSION = 10;
+ // We apply Java 7 class file constraints on DEX files.
+ public static final int CORRESPONDING_CLASS_FILE_VERSION = 51;
+
public static final int DEX_MAGIC_SIZE = 8;
public static final int MAGIC_OFFSET = 0;
diff --git a/src/main/java/com/android/tools/r8/dex/DexFileReader.java b/src/main/java/com/android/tools/r8/dex/DexFileReader.java
index e3559fe..812273b 100644
--- a/src/main/java/com/android/tools/r8/dex/DexFileReader.java
+++ b/src/main/java/com/android/tools/r8/dex/DexFileReader.java
@@ -10,6 +10,7 @@
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.code.InstructionFactory;
+import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.graph.ClassAccessFlags;
import com.android.tools.r8.graph.ClassKind;
import com.android.tools.r8.graph.Descriptor;
@@ -637,10 +638,14 @@
DexType superclass = superclassIdx == NO_INDEX ? null : indexedItems.getType(superclassIdx);
int srcIdx = sourceFileIndices[i];
DexString source = srcIdx == NO_INDEX ? null : indexedItems.getString(srcIdx);
- // fix annotations.
DexType type = indexedItems.getType(classIndices[i]);
ClassAccessFlags flags = ClassAccessFlags.fromDexAccessFlags(accessFlags[i]);
- DexClass clazz;
+ // Check if constraints from
+ // https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1 are met.
+ if (!flags.areValid(Constants.CORRESPONDING_CLASS_FILE_VERSION)) {
+ throw new CompilationError("Class " + type.toSourceString()
+ + " has illegal access flags. Found: " + flags, origin);
+ }
DexEncodedField[] staticFields = DexEncodedField.EMPTY_ARRAY;
DexEncodedField[] instanceFields = DexEncodedField.EMPTY_ARRAY;
DexEncodedMethod[] directMethods = DexEncodedMethod.EMPTY_ARRAY;
@@ -675,22 +680,21 @@
AttributesAndAnnotations attrs =
new AttributesAndAnnotations(type, annotationsDirectory.clazz, dexItemFactory);
- clazz =
- classKind.create(
- type,
- Kind.DEX,
- origin,
- flags,
- superclass,
- typeListAt(interfacesOffsets[i]),
- source,
- attrs.getEnclosingMethodAttribute(),
- attrs.getInnerClasses(),
- attrs.getAnnotations(),
- staticFields,
- instanceFields,
- directMethods,
- virtualMethods);
+ DexClass clazz = classKind.create(
+ type,
+ Kind.DEX,
+ origin,
+ flags,
+ superclass,
+ typeListAt(interfacesOffsets[i]),
+ source,
+ attrs.getEnclosingMethodAttribute(),
+ attrs.getInnerClasses(),
+ attrs.getAnnotations(),
+ staticFields,
+ instanceFields,
+ directMethods,
+ virtualMethods);
classCollection.accept(clazz); // Update the application object.
}
}
@@ -937,7 +941,7 @@
MethodHandleType type = MethodHandleType.getKind(file.getUshort());
file.getUshort(); // unused
int indexFieldOrMethod = file.getUshort();
- Descriptor<? extends DexItem, ? extends Descriptor<?,?>> fieldOrMethod;
+ Descriptor<? extends DexItem, ? extends Descriptor<?, ?>> fieldOrMethod;
switch (type) {
case INSTANCE_GET:
case INSTANCE_PUT:
diff --git a/src/main/java/com/android/tools/r8/errors/CompilationError.java b/src/main/java/com/android/tools/r8/errors/CompilationError.java
index 060699e..4409816 100644
--- a/src/main/java/com/android/tools/r8/errors/CompilationError.java
+++ b/src/main/java/com/android/tools/r8/errors/CompilationError.java
@@ -3,11 +3,13 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.errors;
+import com.android.tools.r8.origin.Origin;
+
/**
* Exception to signal an compilation error.
- *
- * This is always an expected error and considered a user input issue.
- * A user-understandable message must be provided.
+ * <p>
+ * This is always an expected error and considered a user input issue. A user-understandable message
+ * must be provided.
*/
public class CompilationError extends RuntimeException {
@@ -18,4 +20,8 @@
public CompilationError(String message, Throwable cause) {
super(message, cause);
}
+
+ public CompilationError(String message, Origin origin) {
+ super(origin + ": " + message);
+ }
}
\ No newline at end of file
diff --git a/src/main/java/com/android/tools/r8/graph/AccessFlags.java b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
index 9f0471e..e2654aa 100644
--- a/src/main/java/com/android/tools/r8/graph/AccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/AccessFlags.java
@@ -166,20 +166,26 @@
public String toSmaliString() {
- return toString();
+ return toStringInternal(true);
}
@Override
public String toString() {
+ return toStringInternal(false);
+ }
+
+ private String toStringInternal(boolean ignoreSuper) {
List<String> names = getNames();
List<BooleanSupplier> predicates = getPredicates();
StringBuilder builder = new StringBuilder();
for (int i = 0; i < names.size(); i++) {
if (predicates.get(i).getAsBoolean()) {
- if (builder.length() > 0) {
- builder.append(' ');
+ if (!ignoreSuper || !names.get(i).equals("super")) {
+ if (builder.length() > 0) {
+ builder.append(' ');
+ }
+ builder.append(names.get(i));
}
- builder.append(names.get(i));
}
}
return builder.toString();
diff --git a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
index 6430924..a72c1bc 100644
--- a/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
+++ b/src/main/java/com/android/tools/r8/graph/ClassAccessFlags.java
@@ -33,6 +33,7 @@
.add("abstract")
.add("annotation")
.add("enum")
+ .add("super")
.build();
}
@@ -44,6 +45,7 @@
.add(this::isAbstract)
.add(this::isAnnotation)
.add(this::isEnum)
+ .add(this::isSuper)
.build();
}
@@ -80,6 +82,27 @@
return flags;
}
+ /**
+ * Checks whether the constraints from
+ * https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1 are met.
+ */
+ public boolean areValid(int majorVersion) {
+ if (isInterface()) {
+ // We ignore the super flags prior to JDK 9, as so did the VM.
+ if ((majorVersion >= 53) && isSuper()) {
+ return false;
+ }
+ // We require interfaces to be abstract from JDK 7 onwards. Old versions of javac seem to
+ // have produced package-info classes that are interfaces but not abstract.
+ if ((majorVersion >= 51) && (!isAbstract())) {
+ return false;
+ }
+ return !isFinal() && !isEnum();
+ } else {
+ return !isAnnotation() && (!isFinal() || !isAbstract());
+ }
+ }
+
public boolean isInterface() {
return isSet(Constants.ACC_INTERFACE);
}
diff --git a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
index 868ac10..ba8a672 100644
--- a/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
+++ b/src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
@@ -8,6 +8,7 @@
import static org.objectweb.asm.Opcodes.ASM6;
import com.android.tools.r8.dex.Constants;
+import com.android.tools.r8.errors.CompilationError;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.DexValue.DexValueAnnotation;
import com.android.tools.r8.graph.DexValue.DexValueArray;
@@ -31,6 +32,7 @@
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
+import java.util.Objects;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import org.objectweb.asm.AnnotationVisitor;
@@ -100,6 +102,7 @@
}
private static class CreateDexClassVisitor extends ClassVisitor {
+
private final Origin origin;
private final ClassKind classKind;
private final JarApplicationReader application;
@@ -163,6 +166,21 @@
this.version = version;
accessFlags = ClassAccessFlags.fromCfAccessFlags(cleanAccessFlags(access));
type = application.getTypeFromName(name);
+ // Check if constraints from
+ // https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1 are met.
+ if (!accessFlags.areValid(getMajorVersion())) {
+ throw new CompilationError("Illegal class file: Class " + name
+ + " has invalid access flags. Found: " + accessFlags.toString(), origin);
+ }
+ if (superName == null && !name.equals(Constants.JAVA_LANG_OBJECT_NAME)) {
+ throw new CompilationError("Illegal class file: Class " + name
+ + " is missing a super type.", origin);
+ }
+ if (accessFlags.isInterface()
+ && !Objects.equals(superName, Constants.JAVA_LANG_OBJECT_NAME)) {
+ throw new CompilationError("Illegal class file: Interface " + name
+ + " must extend class java.lang.Object. Found: " + Objects.toString(superName), origin);
+ }
assert superName != null || name.equals(Constants.JAVA_LANG_OBJECT_NAME);
superType = superName == null ? null : application.getTypeFromName(superName);
this.interfaces = application.getTypeListFromNames(interfaces);
@@ -178,8 +196,8 @@
}
if (debug != null) {
getAnnotations().add(
- DexAnnotation.createSourceDebugExtensionAnnotation(
- new DexValueString(application.getString(debug)), application.getFactory()));
+ DexAnnotation.createSourceDebugExtensionAnnotation(
+ new DexValueString(application.getString(debug)), application.getFactory()));
}
}
@@ -260,6 +278,14 @@
}
return annotations;
}
+
+ private int getMajorVersion() {
+ return version & 0xFFFF;
+ }
+
+ private int getMinorVersion() {
+ return ((version >> 16) & 0xFFFF);
+ }
}
private static DexAnnotationSet createAnnotationSet(List<DexAnnotation> annotations) {
@@ -269,6 +295,7 @@
}
private static class CreateFieldVisitor extends FieldVisitor {
+
private final CreateDexClassVisitor parent;
private final int access;
private final String name;
@@ -366,6 +393,7 @@
}
private static class CreateMethodVisitor extends MethodVisitor {
+
private final int access;
private final String name;
private final String desc;
@@ -541,6 +569,7 @@
}
private static class CreateAnnotationVisitor extends AnnotationVisitor {
+
private final JarApplicationReader application;
private final BiConsumer<List<DexString>, List<DexValue>> onVisitEnd;
private List<DexString> names = null;
@@ -586,7 +615,7 @@
private void addElement(String name, DexValue value) {
if (name != null) {
- if (names == null){
+ if (names == null) {
names = new ArrayList<>();
}
names.add(application.getString(name));
diff --git a/src/main/java/com/android/tools/r8/utils/DirectoryClassFileProvider.java b/src/main/java/com/android/tools/r8/utils/DirectoryClassFileProvider.java
index eba7f8e..e8ee800 100644
--- a/src/main/java/com/android/tools/r8/utils/DirectoryClassFileProvider.java
+++ b/src/main/java/com/android/tools/r8/utils/DirectoryClassFileProvider.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.utils;
import static com.android.tools.r8.utils.FileUtils.CLASS_EXTENSION;
+import static com.android.tools.r8.utils.FileUtils.isClassFile;
import com.android.tools.r8.ClassFileResourceProvider;
import com.android.tools.r8.Resource;
@@ -41,10 +42,9 @@
if (child.isDirectory()) {
collectClassDescriptors(child.toPath(), result);
} else {
- String relative = root.relativize(child.toPath()).toString();
- if (relative.endsWith(CLASS_EXTENSION)) {
- result.add("L" + relative.substring(
- 0, relative.length() - CLASS_EXTENSION.length()) + ";");
+ Path relative = root.relativize(child.toPath());
+ if (isClassFile(relative)) {
+ result.add(DescriptorUtils.guessTypeDescriptor(relative));
}
}
}
diff --git a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
index 5eb7168..410f281 100644
--- a/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
+++ b/src/test/java/com/android/tools/r8/R8RunArtTestsTest.java
@@ -855,6 +855,9 @@
.put("960-default-smali", TestCondition.match(TestCondition.R8_COMPILER))
.put("966-default-conflict", TestCondition.match(TestCondition.R8_COMPILER))
.put("972-iface-super-multidex", TestCondition.match(TestCondition.R8_COMPILER))
+ // These tests have illegal class flag combinations, so we reject them.
+ .put("161-final-abstract-class", TestCondition.any())
+ .put("004-JniTest", TestCondition.any())
.build();
// Tests that does not have dex input for some toolchains.
diff --git a/src/test/smali/illegal-invokes/Iface.smali b/src/test/smali/illegal-invokes/Iface.smali
index d66777c..5f074dce 100644
--- a/src/test/smali/illegal-invokes/Iface.smali
+++ b/src/test/smali/illegal-invokes/Iface.smali
@@ -2,7 +2,7 @@
# 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.
-.class interface public LIface;
+.class interface public abstract LIface;
.super Ljava/lang/Object;
.method public abstract bar()V
