Read JDWP options from runtime

Allocates JDWP::JdwpOptions on the heap and copies parsed options to
avoid the need to include jdwp/jdwp.h into runtime.h file.

Also does some minor cleanup and removes the old JDWP options parsing
code that became dead code after we move it to the new command-line
parser.

Bug: 19275792
Change-Id: I71901c89fbf2cc3c1901a089e2a98b4326c6ee70

6 files changed, 63 insertions, 140 deletions

diff --git a/cmdline/cmdline_types.h b/cmdline/cmdline_types.h
index 92210235e8..45f9b5635c 100644
--- a/cmdline/cmdline_types.h
+++ b/cmdline/cmdline_types.h
@@ -56,6 +56,23 @@ struct CmdlineType<Unit> : CmdlineTypeParser<Unit> {
 
 template <>
 struct CmdlineType<JDWP::JdwpOptions> : CmdlineTypeParser<JDWP::JdwpOptions> {
+  /*
+   * Handle one of the JDWP name/value pairs.
+   *
+   * JDWP options are:
+   *  help: if specified, show help message and bail
+   *  transport: may be dt_socket or dt_shmem
+   *  address: for dt_socket, "host:port", or just "port" when listening
+   *  server: if "y", wait for debugger to attach; if "n", attach to debugger
+   *  timeout: how long to wait for debugger to connect / listen
+   *
+   * Useful with server=n (these aren't supported yet):
+   *  onthrow=<exception-name>: connect to debugger when exception thrown
+   *  onuncaught=y|n: connect to debugger when uncaught exception thrown
+   *  launch=<command-line>: launch the debugger itself
+   *
+   * The "transport" option is required, as is "address" if server=n.
+   */
   Result Parse(const std::string& options) {
     VLOG(jdwp) << "ParseJdwpOptions: " << options;
 
@@ -70,20 +87,20 @@ struct CmdlineType<JDWP::JdwpOptions> : CmdlineTypeParser<JDWP::JdwpOptions> {
     std::vector<std::string> pairs;
     Split(options, ',', &pairs);
 
-    JDWP::JdwpOptions jdwp_options = JDWP::JdwpOptions();
+    JDWP::JdwpOptions jdwp_options;
     std::stringstream error_stream;
 
-    for (size_t i = 0; i < pairs.size(); ++i) {
-      std::string::size_type equals = pairs[i].find('=');
-      if (equals == std::string::npos) {
+    for (const std::string& jdwp_option : pairs) {
+      std::string::size_type equals_pos = jdwp_option.find('=');
+      if (equals_pos == std::string::npos) {
         return Result::Failure(s +
-            "Can't parse JDWP option '" + pairs[i] + "' in '" + options + "'");
+            "Can't parse JDWP option '" + jdwp_option + "' in '" + options + "'");
       }
 
-      if (!ParseJdwpOption(pairs[i].substr(0, equals),
-                           pairs[i].substr(equals + 1),
+      if (!ParseJdwpOption(jdwp_option.substr(0, equals_pos),
+                           jdwp_option.substr(equals_pos + 1),
                            error_stream,
-                           jdwp_options)) {
+                           &jdwp_options)) {
         return Result::Failure(error_stream.str());
       }
     }
@@ -100,30 +117,30 @@ struct CmdlineType<JDWP::JdwpOptions> : CmdlineTypeParser<JDWP::JdwpOptions> {
 
   bool ParseJdwpOption(const std::string& name, const std::string& value,
                        std::ostream& error_stream,
-                       JDWP::JdwpOptions& jdwp_options) {
+                       JDWP::JdwpOptions* jdwp_options) {
     if (name == "transport") {
       if (value == "dt_socket") {
-        jdwp_options.transport = JDWP::kJdwpTransportSocket;
+        jdwp_options->transport = JDWP::kJdwpTransportSocket;
       } else if (value == "dt_android_adb") {
-        jdwp_options.transport = JDWP::kJdwpTransportAndroidAdb;
+        jdwp_options->transport = JDWP::kJdwpTransportAndroidAdb;
       } else {
         error_stream << "JDWP transport not supported: " << value;
         return false;
       }
     } else if (name == "server") {
       if (value == "n") {
-        jdwp_options.server = false;
+        jdwp_options->server = false;
       } else if (value == "y") {
-        jdwp_options.server = true;
+        jdwp_options->server = true;
       } else {
         error_stream << "JDWP option 'server' must be 'y' or 'n'";
         return false;
       }
     } else if (name == "suspend") {
       if (value == "n") {
-        jdwp_options.suspend = false;
+        jdwp_options->suspend = false;
       } else if (value == "y") {
-        jdwp_options.suspend = true;
+        jdwp_options->suspend = true;
       } else {
         error_stream << "JDWP option 'suspend' must be 'y' or 'n'";
         return false;
@@ -131,10 +148,10 @@ struct CmdlineType<JDWP::JdwpOptions> : CmdlineTypeParser<JDWP::JdwpOptions> {
     } else if (name == "address") {
       /* this is either <port> or <host>:<port> */
       std::string port_string;
-      jdwp_options.host.clear();
+      jdwp_options->host.clear();
       std::string::size_type colon = value.find(':');
       if (colon != std::string::npos) {
-        jdwp_options.host = value.substr(0, colon);
+        jdwp_options->host = value.substr(0, colon);
         port_string = value.substr(colon + 1);
       } else {
         port_string = value;
@@ -149,7 +166,7 @@ struct CmdlineType<JDWP::JdwpOptions> : CmdlineTypeParser<JDWP::JdwpOptions> {
         error_stream << "JDWP address has junk in port field: " << value;
         return false;
       }
-      jdwp_options.port = port;
+      jdwp_options->port = port;
     } else if (name == "launch" || name == "onthrow" || name == "oncaught" || name == "timeout") {
       /* valid but unsupported */
       LOG(INFO) << "Ignoring JDWP option '" << name << "'='" << value << "'";
diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index a0e978b7e0..678b29a51a 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -297,9 +297,6 @@ static bool gJdwpAllowed = true;
 // Was there a -Xrunjdwp or -agentlib:jdwp= argument on the command line?
 static bool gJdwpConfigured = false;
 
-// Broken-down JDWP options. (Only valid if IsJdwpConfigured() is true.)
-static JDWP::JdwpOptions gJdwpOptions;
-
 // Runtime JDWP state.
 static JDWP::JdwpState* gJdwpState = nullptr;
 static bool gDebuggerConnected;  // debugger or DDMS is connected.
@@ -532,119 +529,13 @@ static bool IsPrimitiveTag(JDWP::JdwpTag tag) {
   }
 }
 
-/*
- * Handle one of the JDWP name/value pairs.
- *
- * JDWP options are:
- *  help: if specified, show help message and bail
- *  transport: may be dt_socket or dt_shmem
- *  address: for dt_socket, "host:port", or just "port" when listening
- *  server: if "y", wait for debugger to attach; if "n", attach to debugger
- *  timeout: how long to wait for debugger to connect / listen
- *
- * Useful with server=n (these aren't supported yet):
- *  onthrow=<exception-name>: connect to debugger when exception thrown
- *  onuncaught=y|n: connect to debugger when uncaught exception thrown
- *  launch=<command-line>: launch the debugger itself
- *
- * The "transport" option is required, as is "address" if server=n.
- */
-static bool ParseJdwpOption(const std::string& name, const std::string& value) {
-  if (name == "transport") {
-    if (value == "dt_socket") {
-      gJdwpOptions.transport = JDWP::kJdwpTransportSocket;
-    } else if (value == "dt_android_adb") {
-      gJdwpOptions.transport = JDWP::kJdwpTransportAndroidAdb;
-    } else {
-      LOG(ERROR) << "JDWP transport not supported: " << value;
-      return false;
-    }
-  } else if (name == "server") {
-    if (value == "n") {
-      gJdwpOptions.server = false;
-    } else if (value == "y") {
-      gJdwpOptions.server = true;
-    } else {
-      LOG(ERROR) << "JDWP option 'server' must be 'y' or 'n'";
-      return false;
-    }
-  } else if (name == "suspend") {
-    if (value == "n") {
-      gJdwpOptions.suspend = false;
-    } else if (value == "y") {
-      gJdwpOptions.suspend = true;
-    } else {
-      LOG(ERROR) << "JDWP option 'suspend' must be 'y' or 'n'";
-      return false;
-    }
-  } else if (name == "address") {
-    /* this is either <port> or <host>:<port> */
-    std::string port_string;
-    gJdwpOptions.host.clear();
-    std::string::size_type colon = value.find(':');
-    if (colon != std::string::npos) {
-      gJdwpOptions.host = value.substr(0, colon);
-      port_string = value.substr(colon + 1);
-    } else {
-      port_string = value;
-    }
-    if (port_string.empty()) {
-      LOG(ERROR) << "JDWP address missing port: " << value;
-      return false;
-    }
-    char* end;
-    uint64_t port = strtoul(port_string.c_str(), &end, 10);
-    if (*end != '\0' || port > 0xffff) {
-      LOG(ERROR) << "JDWP address has junk in port field: " << value;
-      return false;
-    }
-    gJdwpOptions.port = port;
-  } else if (name == "launch" || name == "onthrow" || name == "oncaught" || name == "timeout") {
-    /* valid but unsupported */
-    LOG(INFO) << "Ignoring JDWP option '" << name << "'='" << value << "'";
-  } else {
-    LOG(INFO) << "Ignoring unrecognized JDWP option '" << name << "'='" << value << "'";
-  }
-
-  return true;
-}
-
-/*
- * Parse the latter half of a -Xrunjdwp/-agentlib:jdwp= string, e.g.:
- * "transport=dt_socket,address=8000,server=y,suspend=n"
- */
-bool Dbg::ParseJdwpOptions(const std::string& options) {
-  VLOG(jdwp) << "ParseJdwpOptions: " << options;
-
-  std::vector<std::string> pairs;
-  Split(options, ',', &pairs);
-
-  for (size_t i = 0; i < pairs.size(); ++i) {
-    std::string::size_type equals = pairs[i].find('=');
-    if (equals == std::string::npos) {
-      LOG(ERROR) << "Can't parse JDWP option '" << pairs[i] << "' in '" << options << "'";
-      return false;
-    }
-    ParseJdwpOption(pairs[i].substr(0, equals), pairs[i].substr(equals + 1));
-  }
-
-  if (gJdwpOptions.transport == JDWP::kJdwpTransportUnknown) {
-    LOG(ERROR) << "Must specify JDWP transport: " << options;
-  }
-  if (!gJdwpOptions.server && (gJdwpOptions.host.empty() || gJdwpOptions.port == 0)) {
-    LOG(ERROR) << "Must specify JDWP host and port when server=n: " << options;
-    return false;
-  }
-
-  gJdwpConfigured = true;
-  return true;
-}
-
-void Dbg::StartJdwp() {
+void Dbg::StartJdwp(const JDWP::JdwpOptions* jdwp_options) {
+  gJdwpConfigured = (jdwp_options != nullptr);
   if (!gJdwpAllowed || !IsJdwpConfigured()) {
     // No JDWP for you!
     return;
   }
+  DCHECK_NE(jdwp_options->transport, JDWP::kJdwpTransportUnknown);
 
   CHECK(gRegistry == nullptr);
   gRegistry = new ObjectRegistry;
@@ -652,7 +543,7 @@ void Dbg::StartJdwp() {
   // Init JDWP if the debugger is enabled. This may connect out to a
   // debugger, passively listen for a debugger, or block waiting for a
   // debugger.
-  gJdwpState = JDWP::JdwpState::Create(&gJdwpOptions);
+  gJdwpState = JDWP::JdwpState::Create(jdwp_options);
   if (gJdwpState == nullptr) {
     // We probably failed because some other process has the port already, which means that
     // if we don't abort the user is likely to think they're talking to us when they're actually
diff --git a/runtime/debugger.h b/runtime/debugger.h
index 901d3e7989..6762608cf2 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -226,10 +226,9 @@ class Dbg {
     std::multimap<int32_t, jobject> objects_;
   };
 
-  static bool ParseJdwpOptions(const std::string& options);
   static void SetJdwpAllowed(bool allowed);
 
-  static void StartJdwp();
+  static void StartJdwp(const JDWP::JdwpOptions* jdwp_options);
   static void StopJdwp();
 
   // Invoked by the GC in case we need to keep DDMS informed.
diff --git a/runtime/jdwp/jdwp.h b/runtime/jdwp/jdwp.h
index 6464a623c2..9f379989a7 100644
--- a/runtime/jdwp/jdwp.h
+++ b/runtime/jdwp/jdwp.h
@@ -101,11 +101,11 @@ enum JdwpTransportType {
 std::ostream& operator<<(std::ostream& os, const JdwpTransportType& rhs);
 
 struct JdwpOptions {
-  JdwpTransportType transport;
-  bool server;
-  bool suspend;
-  std::string host;
-  uint16_t port;
+  JdwpTransportType transport = kJdwpTransportUnknown;
+  bool server = false;
+  bool suspend = false;
+  std::string host = "";
+  uint16_t port = static_cast<uint16_t>(-1);
 };
 
 bool operator==(const JdwpOptions& lhs, const JdwpOptions& rhs);
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 43f3a2e926..e868e752c1 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -173,7 +173,8 @@ Runtime::Runtime()
       implicit_null_checks_(false),
       implicit_so_checks_(false),
       implicit_suspend_checks_(false),
-      is_native_bridge_loaded_(false) {
+      is_native_bridge_loaded_(false),
+      jdwp_options_(nullptr) {
   CheckAsmSupportOffsetsAndSizes();
 }
 
@@ -227,6 +228,10 @@ Runtime::~Runtime() {
 
   // Make sure our internal threads are dead before we start tearing down things they're using.
   Dbg::StopJdwp();
+  if (jdwp_options_ != nullptr) {
+    delete jdwp_options_;
+  }
+
   delete signal_catcher_;
 
   // Make sure all other non-daemon threads have terminated, and all daemon threads are suspended.
@@ -590,7 +595,7 @@ void Runtime::DidForkFromZygote(JNIEnv* env, NativeBridgeAction action, const ch
 
   // Start the JDWP thread. If the command-line debugger flags specified "suspend=y",
   // this will pause the runtime, so we probably want this to come last.
-  Dbg::StartJdwp();
+  Dbg::StartJdwp(jdwp_options_);
 }
 
 void Runtime::StartSignalCatcher() {
@@ -799,6 +804,11 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
 
   dump_gc_performance_on_shutdown_ = runtime_options.Exists(Opt::DumpGCPerformanceOnShutdown);
 
+  if (runtime_options.Exists(Opt::JdwpOptions)) {
+    JDWP::JdwpOptions options = runtime_options.GetOrDefault(Opt::JdwpOptions);
+    jdwp_options_ = new JDWP::JdwpOptions(options);
+  }
+
   BlockSignals();
   InitPlatformSignalHandlers();
 
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 118c838397..d98eb8d5d2 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -47,6 +47,9 @@ namespace gc {
     class GarbageCollector;
   }  // namespace collector
 }  // namespace gc
+namespace JDWP {
+  struct JdwpOptions;
+}  // namespace JDWP
 namespace mirror {
   class ArtMethod;
   class ClassLoader;
@@ -682,6 +685,9 @@ class Runtime {
   // that there's no native bridge.
   bool is_native_bridge_loaded_;
 
+  // JDWP options for debugging.
+  const JDWP::JdwpOptions* jdwp_options_;
+
   DISALLOW_COPY_AND_ASSIGN(Runtime);
 };
 std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs);