Fix index checks for error strings in DexFileVerifier.
Bug: 28552165
Change-Id: I5b352536b746ec5bd48daa5c693d62d9fbc0b21c
diff --git a/runtime/dex_file_verifier_test.cc b/runtime/dex_file_verifier_test.cc
index 344d186..84f48da 100644
--- a/runtime/dex_file_verifier_test.cc
+++ b/runtime/dex_file_verifier_test.cc
@@ -57,7 +57,7 @@
255, 255, 255, 255
};
-static inline uint8_t* DecodeBase64(const char* src, size_t* dst_size) {
+static inline std::unique_ptr<uint8_t[]> DecodeBase64(const char* src, size_t* dst_size) {
std::vector<uint8_t> tmp;
uint32_t t = 0, y = 0;
int g = 3;
@@ -100,7 +100,7 @@
*dst_size = 0;
}
std::copy(tmp.begin(), tmp.end(), dst.get());
- return dst.release();
+ return dst;
}
static void FixUpChecksum(uint8_t* dex_file) {
@@ -113,25 +113,18 @@
header->checksum_ = adler_checksum;
}
-// Custom deleter. Necessary to clean up the memory we use (to be able to mutate).
-struct DexFileDeleter {
- void operator()(DexFile* in) {
- if (in != nullptr) {
- delete[] in->Begin();
- delete in;
- }
- }
-};
-
-using DexFileUniquePtr = std::unique_ptr<DexFile, DexFileDeleter>;
-
class DexFileVerifierTest : public CommonRuntimeTest {
protected:
void VerifyModification(const char* dex_file_base64_content,
const char* location,
std::function<void(DexFile*)> f,
const char* expected_error) {
- DexFileUniquePtr dex_file(WrapAsDexFile(dex_file_base64_content));
+ size_t length;
+ std::unique_ptr<uint8_t[]> dex_bytes = DecodeBase64(dex_file_base64_content, &length);
+ CHECK(dex_bytes != nullptr);
+ // Note: `dex_file` will be destroyed before `dex_bytes`.
+ std::unique_ptr<DexFile> dex_file(
+ new DexFile(dex_bytes.get(), length, "tmp", 0, nullptr, nullptr));
f(dex_file.get());
FixUpChecksum(const_cast<uint8_t*>(dex_file->Begin()));
@@ -150,15 +143,6 @@
}
}
}
-
- private:
- static DexFile* WrapAsDexFile(const char* dex_file_content_in_base_64) {
- // Decode base64.
- size_t length;
- uint8_t* dex_bytes = DecodeBase64(dex_file_content_in_base_64, &length);
- CHECK(dex_bytes != nullptr);
- return new DexFile(dex_bytes, length, "tmp", 0, nullptr, nullptr);
- }
};
static std::unique_ptr<const DexFile> OpenDexFileBase64(const char* base64,
@@ -290,7 +274,9 @@
// Find the method data for the first method with the given name (from class 0). Note: the pointer
// is to the access flags, so that the caller doesn't have to handle the leb128-encoded method-index
// delta.
-static const uint8_t* FindMethodData(const DexFile* dex_file, const char* name) {
+static const uint8_t* FindMethodData(const DexFile* dex_file,
+ const char* name,
+ /*out*/ uint32_t* method_idx = nullptr) {
const DexFile::ClassDef& class_def = dex_file->GetClassDef(0);
const uint8_t* class_data = dex_file->GetClassData(class_def);
@@ -316,6 +302,9 @@
const DexFile::StringId& string_id = dex_file->GetStringId(name_index);
const char* str = dex_file->GetStringData(string_id);
if (strcmp(name, str) == 0) {
+ if (method_idx != nullptr) {
+ *method_idx = method_index;
+ }
DecodeUnsignedLeb128(&trailing);
return trailing;
}
@@ -683,6 +672,22 @@
}
}
+TEST_F(DexFileVerifierTest, B28552165) {
+ // Regression test for bad error string retrieval in different situations.
+ // Using invalid access flags to trigger the error.
+ VerifyModification(
+ kMethodFlagsTestDex,
+ "b28552165",
+ [](DexFile* dex_file) {
+ OrMaskToMethodFlags(dex_file, "foo", kAccPublic | kAccProtected);
+ uint32_t method_idx;
+ FindMethodData(dex_file, "foo", &method_idx);
+ auto* method_id = const_cast<DexFile::MethodId*>(&dex_file->GetMethodId(method_idx));
+ method_id->name_idx_ = dex_file->NumStringIds();
+ },
+ "Method may have only one of public/protected/private, LMethodFlags;.(error)");
+}
+
// Set of dex files for interface method tests. As it's not as easy to mutate method names, it's
// just easier to break up bad cases.