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

(cherry picked from commit aabbb2066a715b3fd8e752291f74c6d77b970450)

Change-Id: I24131914bcf91c04ae93179bf809a2907f1f2b7a

diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index 72ad9a5..bbdf3a3 100644 (file)
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -823,22 +823,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 d8a38f4..644e055 100644 (file)
--- 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 f212811..ae8eeac 100644 (file)
--- 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 ece3238..ac17c4f 100644 (file)
--- 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 f199c99..14d6cd9 100644 (file)
--- 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<jclass> 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 a023ff1..26360d7 100644 (file)
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -414,8 +414,12 @@ bool ParsedOptions::Parse(const RuntimeOptions& options, bool ignore_unrecognize
       compiler_callbacks_ =
           reinterpret_cast<CompilerCallbacks*>(const_cast<void*>(options[i].second));
     } else if (option == "imageinstructionset") {
-      image_isa_ = GetInstructionSetFromString(
-          reinterpret_cast<const char*>(options[i].second));
+      const char* isa_str = reinterpret_cast<const char*>(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:")) {