ART: Postpone interface-related dex failure to version 37
For app compat, at least for now make the check for public-final-static
of interface members not fail on dex file versions less than 37. This
may be changed in future releases.
Bug: 27831184
Change-Id: If8ee50321298b951d4a78062c8eb583fec27394f
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 60caa73..e63eaa2 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -506,8 +506,8 @@
return false;
}
-uint32_t DexFile::GetVersion() const {
- const char* version = reinterpret_cast<const char*>(&GetHeader().magic_[sizeof(kDexMagic)]);
+uint32_t DexFile::Header::GetVersion() const {
+ const char* version = reinterpret_cast<const char*>(&magic_[sizeof(kDexMagic)]);
return atoi(version);
}
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index 6849984..1456636 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -107,6 +107,9 @@
uint32_t data_size_; // unused
uint32_t data_off_; // unused
+ // Decode the dex magic version
+ uint32_t GetVersion() const;
+
private:
DISALLOW_COPY_AND_ASSIGN(Header);
};
@@ -479,7 +482,9 @@
}
// Decode the dex magic version
- uint32_t GetVersion() const;
+ uint32_t GetVersion() const {
+ return GetHeader().GetVersion();
+ }
// Returns true if the byte string points to the magic value.
static bool IsMagicValid(const uint8_t* magic);
diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc
index 9c9b8c5..811e763 100644
--- a/runtime/dex_file_verifier.cc
+++ b/runtime/dex_file_verifier.cc
@@ -2465,7 +2465,13 @@
GetFieldDescriptionOrError(begin_, header_, idx).c_str(),
field_access_flags,
PrettyJavaAccessFlags(field_access_flags).c_str());
- return false;
+ if (header_->GetVersion() >= 37) {
+ return false;
+ } else {
+ // Allow in older versions, but warn.
+ LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: "
+ << *error_msg;
+ }
}
// Interface fields may be synthetic, but may not have other flags.
constexpr uint32_t kDisallowed = ~(kPublicFinalStatic | kAccSynthetic);
@@ -2474,7 +2480,13 @@
GetFieldDescriptionOrError(begin_, header_, idx).c_str(),
field_access_flags,
PrettyJavaAccessFlags(field_access_flags).c_str());
- return false;
+ if (header_->GetVersion() >= 37) {
+ return false;
+ } else {
+ // Allow in older versions, but warn.
+ LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: "
+ << *error_msg;
+ }
}
return true;
}
@@ -2657,7 +2669,13 @@
*error_msg = StringPrintf("Interface method %" PRIu32 "(%s) is not public and abstract",
method_index,
GetMethodDescriptionOrError(begin_, header_, method_index).c_str());
- return false;
+ if (header_->GetVersion() >= 37) {
+ return false;
+ } else {
+ // Allow in older versions, but warn.
+ LOG(WARNING) << "This dex file is invalid and will be rejected in the future. Error is: "
+ << *error_msg;
+ }
}
// At this point, we know the method is public and abstract. This means that all the checks
// for invalid combinations above applies. In addition, interface methods must not be
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 44cf2ee..4a5ed5d 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -725,6 +725,13 @@
return result;
}
+// Make the Dex file version 37.
+static void MakeDexVersion37(DexFile* dex_file) {
+ size_t offset = OFFSETOF_MEMBER(DexFile::Header, magic_) + 6;
+ CHECK_EQ(*(dex_file->Begin() + offset), '5');
+ *(const_cast<uint8_t*>(dex_file->Begin()) + offset) = '7';
+}
+
TEST_F(DexFileVerifierTest, MethodAccessFlagsInterfaces) {
VerifyModification(
kMethodFlagsInterface,
@@ -733,6 +740,14 @@
ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
},
nullptr);
+ VerifyModification(
+ kMethodFlagsInterface,
+ "method_flags_interface_ok37",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+ },
+ nullptr);
VerifyModification(
kMethodFlagsInterface,
@@ -742,7 +757,18 @@
ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kMethodFlagsInterface,
+ "method_flags_interface_non_public",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
+ },
"Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
+
VerifyModification(
kMethodFlagsInterface,
"method_flags_interface_non_abstract",
@@ -781,7 +807,18 @@
ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kMethodFlagsInterface,
+ "method_flags_interface_non_public",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
+ },
"Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
+
VerifyModification(
kMethodFlagsInterface,
"method_flags_interface_protected",
@@ -791,6 +828,17 @@
ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
OrMaskToMethodFlags(dex_file, "foo", kAccProtected);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kMethodFlagsInterface,
+ "method_flags_interface_protected",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToMethodFlags(dex_file, "foo", ~kAccPublic);
+ OrMaskToMethodFlags(dex_file, "foo", kAccProtected);
+ },
"Interface method 1(LInterfaceMethodFlags;.foo) is not public and abstract");
constexpr uint32_t kAllMethodFlags =
@@ -1070,6 +1118,14 @@
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
},
nullptr);
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+ },
+ nullptr);
VerifyModification(
kFieldFlagsInterfaceTestDex,
@@ -1079,7 +1135,18 @@
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface_non_public",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
+ },
"Interface field is not public final static");
+
VerifyModification(
kFieldFlagsInterfaceTestDex,
"field_flags_interface_non_final",
@@ -1088,7 +1155,18 @@
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccFinal);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface_non_final",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccFinal);
+ },
"Interface field is not public final static");
+
VerifyModification(
kFieldFlagsInterfaceTestDex,
"field_flags_interface_protected",
@@ -1098,7 +1176,19 @@
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
OrMaskToFieldFlags(dex_file, "foo", kAccProtected);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface_protected",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
+ OrMaskToFieldFlags(dex_file, "foo", kAccProtected);
+ },
"Interface field is not public final static");
+
VerifyModification(
kFieldFlagsInterfaceTestDex,
"field_flags_interface_private",
@@ -1108,6 +1198,17 @@
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
OrMaskToFieldFlags(dex_file, "foo", kAccPrivate);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface_private",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
+ OrMaskToFieldFlags(dex_file, "foo", kAccPrivate);
+ },
"Interface field is not public final static");
VerifyModification(
@@ -1152,6 +1253,21 @@
}
OrMaskToFieldFlags(dex_file, "foo", mask);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceTestDex,
+ "field_flags_interface_disallowed",
+ [&](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+
+ uint32_t mask = ApplyMaskShifted(kInterfaceDisallowed, i);
+ if ((mask & kAccProtected) != 0) {
+ mask &= ~kAccProtected;
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccPublic);
+ }
+ OrMaskToFieldFlags(dex_file, "foo", mask);
+ },
"Interface field has disallowed flag");
}
}
@@ -1180,6 +1296,14 @@
[](DexFile* dex_file) {
ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
},
+ nullptr); // Should be allowed in older dex versions for backwards compatibility.
+ VerifyModification(
+ kFieldFlagsInterfaceBadTestDex,
+ "field_flags_interface_non_static",
+ [](DexFile* dex_file) {
+ MakeDexVersion37(dex_file);
+ ApplyMaskToFieldFlags(dex_file, "foo", ~kAccDeclaredSynchronized);
+ },
"Interface field is not public final static");
}
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 3d5f84e..83da6b7 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -724,13 +724,15 @@
// If there aren't any instructions, make sure that's expected, then exit successfully.
if (code_item_ == nullptr) {
+ // Only native or abstract methods may not have code.
+ if ((method_access_flags_ & (kAccNative | kAccAbstract)) == 0) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "zero-length code in concrete non-native method";
+ return false;
+ }
+
// This should have been rejected by the dex file verifier. Only do in debug build.
+ // Note: the above will also be rejected in the dex file verifier, starting in dex version 37.
if (kIsDebugBuild) {
- // Only native or abstract methods may not have code.
- if ((method_access_flags_ & (kAccNative | kAccAbstract)) == 0) {
- Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "zero-length code in concrete non-native method";
- return false;
- }
if ((method_access_flags_ & kAccAbstract) != 0) {
// Abstract methods are not allowed to have the following flags.
static constexpr uint32_t kForbidden =