From aabbb2066a715b3fd8e752291f74c6d77b970450 Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Tue, 19 Aug 2014 17:28:06 -0700 Subject: [PATCH] ART: Relax GetInstructionSetFromString Do not abort on an unknown instruction set string. Instead return kNone and let the caller handle this. Also simplify the patchoat tool to use this. Bug: 17136416 Change-Id: I24131914bcf91c04ae93179bf809a2907f1f2b7a --- patchoat/patchoat.cc | 16 +++------------- runtime/instruction_set.cc | 13 +++++-------- runtime/instruction_set.h | 2 ++ runtime/instruction_set_test.cc | 1 + runtime/native/dalvik_system_DexFile.cc | 15 +++++++++++++++ runtime/parsed_options.cc | 8 ++++++-- 6 files changed, 32 insertions(+), 23 deletions(-) diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc index 72189c340..55e81415b 100644 --- a/patchoat/patchoat.cc +++ b/patchoat/patchoat.cc @@ -804,22 +804,12 @@ static int patchoat(int argc, char **argv) { if (log_options) { LOG(INFO) << "patchoat: option[" << i << "]=" << argv[i]; } - // TODO: GetInstructionSetFromString shouldn't LOG(FATAL). if (option.starts_with("--instruction-set=")) { isa_set = true; const char* isa_str = option.substr(strlen("--instruction-set=")).data(); - if (!strcmp("arm", isa_str)) { - isa = kArm; - } else if (!strcmp("arm64", isa_str)) { - isa = kArm64; - } else if (!strcmp("x86", isa_str)) { - isa = kX86; - } else if (!strcmp("x86_64", isa_str)) { - isa = kX86_64; - } else if (!strcmp("mips", isa_str)) { - isa = kMips; - } else { - Usage("Unknown instruction set %s", isa_str); + isa = GetInstructionSetFromString(isa_str); + if (isa == kNone) { + Usage("Unknown or invalid instruction set %s", isa_str); } } else if (option.starts_with("--input-oat-location=")) { if (have_input_oat) { diff --git a/runtime/instruction_set.cc b/runtime/instruction_set.cc index d8a38f42d..644e0556b 100644 --- a/runtime/instruction_set.cc +++ b/runtime/instruction_set.cc @@ -42,21 +42,18 @@ const char* GetInstructionSetString(const InstructionSet isa) { InstructionSet GetInstructionSetFromString(const char* isa_str) { CHECK(isa_str != nullptr); - if (!strcmp("arm", isa_str)) { + if (strcmp("arm", isa_str) == 0) { return kArm; - } else if (!strcmp("arm64", isa_str)) { + } else if (strcmp("arm64", isa_str) == 0) { return kArm64; - } else if (!strcmp("x86", isa_str)) { + } else if (strcmp("x86", isa_str) == 0) { return kX86; - } else if (!strcmp("x86_64", isa_str)) { + } else if (strcmp("x86_64", isa_str) == 0) { return kX86_64; - } else if (!strcmp("mips", isa_str)) { + } else if (strcmp("mips", isa_str) == 0) { return kMips; - } else if (!strcmp("none", isa_str)) { - return kNone; } - LOG(FATAL) << "Unknown ISA " << isa_str; return kNone; } diff --git a/runtime/instruction_set.h b/runtime/instruction_set.h index f212811e3..ae8eeac54 100644 --- a/runtime/instruction_set.h +++ b/runtime/instruction_set.h @@ -75,6 +75,8 @@ static constexpr size_t kX86Alignment = 16; const char* GetInstructionSetString(InstructionSet isa); + +// Note: Returns kNone when the string cannot be parsed to a known value. InstructionSet GetInstructionSetFromString(const char* instruction_set); static inline size_t GetInstructionSetPointerSize(InstructionSet isa) { diff --git a/runtime/instruction_set_test.cc b/runtime/instruction_set_test.cc index ece32386d..ac17c4f0d 100644 --- a/runtime/instruction_set_test.cc +++ b/runtime/instruction_set_test.cc @@ -29,6 +29,7 @@ TEST_F(InstructionSetTest, GetInstructionSetFromString) { EXPECT_EQ(kX86_64, GetInstructionSetFromString("x86_64")); EXPECT_EQ(kMips, GetInstructionSetFromString("mips")); EXPECT_EQ(kNone, GetInstructionSetFromString("none")); + EXPECT_EQ(kNone, GetInstructionSetFromString("random-string")); } TEST_F(InstructionSetTest, GetInstructionSetString) { diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc index f199c9959..14d6cd950 100644 --- a/runtime/native/dalvik_system_DexFile.cc +++ b/runtime/native/dalvik_system_DexFile.cc @@ -27,6 +27,7 @@ #include "base/logging.h" #include "base/stl_util.h" +#include "base/stringprintf.h" #include "class_linker.h" #include "common_throws.h" #include "dex_file-inl.h" @@ -490,6 +491,12 @@ static jbyte IsDexOptNeededInternal(JNIEnv* env, const char* filename, } const InstructionSet target_instruction_set = GetInstructionSetFromString(instruction_set); + if (target_instruction_set == kNone) { + ScopedLocalRef iae(env, env->FindClass("java/lang/IllegalArgumentException")); + std::string message(StringPrintf("Instruction set %s is invalid.", instruction_set)); + env->ThrowNew(iae.get(), message.c_str()); + return 0; + } // Get the filename for odex file next to the dex file. std::string odex_filename(DexFilenameToOdexFilename(filename, target_instruction_set)); @@ -551,8 +558,16 @@ static jbyte IsDexOptNeededInternal(JNIEnv* env, const char* filename, static jbyte DexFile_isDexOptNeededInternal(JNIEnv* env, jclass, jstring javaFilename, jstring javaPkgname, jstring javaInstructionSet, jboolean defer) { ScopedUtfChars filename(env, javaFilename); + if (env->ExceptionCheck()) { + return 0; + } + NullableScopedUtfChars pkgname(env, javaPkgname); + ScopedUtfChars instruction_set(env, javaInstructionSet); + if (env->ExceptionCheck()) { + return 0; + } return IsDexOptNeededInternal(env, filename.c_str(), pkgname.c_str(), instruction_set.c_str(), defer); diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc index b06eb5f94..c236df592 100644 --- a/runtime/parsed_options.cc +++ b/runtime/parsed_options.cc @@ -415,8 +415,12 @@ bool ParsedOptions::Parse(const RuntimeOptions& options, bool ignore_unrecognize compiler_callbacks_ = reinterpret_cast(const_cast(options[i].second)); } else if (option == "imageinstructionset") { - image_isa_ = GetInstructionSetFromString( - reinterpret_cast(options[i].second)); + const char* isa_str = reinterpret_cast(options[i].second); + image_isa_ = GetInstructionSetFromString(isa_str); + if (image_isa_ == kNone) { + Usage("%s is not a valid instruction set.", isa_str); + return false; + } } else if (option == "-Xzygote") { is_zygote_ = true; } else if (StartsWith(option, "-Xpatchoat:")) { -- 2.11.0