JDWP agent parameters manager
Current implementation uses string comparison which makes it hard to
work with parameters. Using a dedicated object improves readability
and maintainability.
This refactor also prepare the codebase to automatically fix filename
to use an absolute "data/data/..." path when needed (see next CL).
Test: NA
Bug: 286291596
Change-Id: I93ce1bfc63cd12a1fb41d64f151afd2c7ff5ef59
diff --git a/adbconnection/Android.bp b/adbconnection/Android.bp
index ecad650..ed8fa38 100644
--- a/adbconnection/Android.bp
+++ b/adbconnection/Android.bp
@@ -28,7 +28,10 @@
cc_defaults {
name: "adbconnection-defaults",
host_supported: true,
- srcs: ["adbconnection.cc"],
+ srcs: [
+ "adbconnection.cc",
+ "jdwpargs.cc",
+ ],
defaults: ["art_defaults"],
// Note that this tool needs to be built for both 32-bit and 64-bit since it requires
diff --git a/adbconnection/adbconnection.cc b/adbconnection/adbconnection.cc
index 239fe31..3ca9166 100644
--- a/adbconnection/adbconnection.cc
+++ b/adbconnection/adbconnection.cc
@@ -14,12 +14,19 @@
* limitations under the License.
*/
+#include "adbconnection.h"
+
+#include <jni.h>
+#include <sys/eventfd.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/uio.h>
+#include <sys/un.h>
+
#include <array>
#include <cstddef>
#include <iterator>
-#include "adbconnection.h"
-
#include "adbconnection/client.h"
#include "android-base/endian.h"
#include "android-base/stringprintf.h"
@@ -32,27 +39,19 @@
#include "base/mutex.h"
#include "base/socket_peer_is_trusted.h"
#include "debugger.h"
+#include "fd_transport.h"
+#include "jdwpargs.h"
#include "jni/java_vm_ext.h"
#include "jni/jni_env_ext.h"
#include "mirror/class-alloc-inl.h"
#include "mirror/throwable.h"
#include "nativehelper/scoped_local_ref.h"
+#include "poll.h"
#include "runtime-inl.h"
#include "runtime_callbacks.h"
#include "scoped_thread_state_change-inl.h"
#include "well_known_classes.h"
-#include "fd_transport.h"
-
-#include "poll.h"
-
-#include <sys/ioctl.h>
-#include <sys/socket.h>
-#include <sys/uio.h>
-#include <sys/un.h>
-#include <sys/eventfd.h>
-#include <jni.h>
-
namespace adbconnection {
static constexpr size_t kJdwpHeaderLen = 11U;
@@ -863,16 +862,29 @@
std::string AdbConnectionState::MakeAgentArg() {
const std::string& opts = art::Runtime::Current()->GetJdwpOptions();
DCHECK(ValidateJdwpOptions(opts));
+
+ VLOG(jdwp) << "Raw jdwp options '" + opts + "'";
+ JdwpArgs parameters = JdwpArgs(opts);
+
+ // See the comment above for why we need to be server=y. Since the agent defaults to server=n
+ // we must always set it.
+ parameters.put("server", "y");
+
+ // See the comment above for why we need to be suspend=n. Since the agent defaults to
+ // suspend=y we must always set it.
+ parameters.put("suspend", "n");
+
+ std::string ddm_already_active = "n";
+ if (notified_ddm_active_) {
+ ddm_already_active = "y";
+ }
+ parameters.put("ddm_already_active", ddm_already_active);
+
+ parameters.put("transport", "dt_fd_forward");
+ parameters.put("address", std::to_string(remote_agent_control_sock_));
+
// TODO Get agent_name_ from something user settable?
- return agent_name_ + "=" + opts + (opts.empty() ? "" : ",") +
- "ddm_already_active=" + (notified_ddm_active_ ? "y" : "n") + "," +
- // See the comment above for why we need to be server=y. Since the agent defaults to server=n
- // we will add it if it wasn't already present for the convenience of the user.
- (ContainsArgument(opts, "server=y") ? "" : "server=y,") +
- // See the comment above for why we need to be suspend=n. Since the agent defaults to
- // suspend=y we will add it if it wasn't already present.
- (ContainsArgument(opts, "suspend=n") ? "" : "suspend=n,") +
- "transport=dt_fd_forward,address=" + std::to_string(remote_agent_control_sock_);
+ return agent_name_ + "=" + parameters.join();
}
void AdbConnectionState::StopDebuggerThreads() {
diff --git a/adbconnection/jdwpargs.cc b/adbconnection/jdwpargs.cc
new file mode 100644
index 0000000..732713e
--- /dev/null
+++ b/adbconnection/jdwpargs.cc
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jdwpargs.h"
+
+#include <algorithm>
+#include <sstream>
+
+#include "base/logging.h" // For VLOG.
+
+namespace adbconnection {
+
+JdwpArgs::JdwpArgs(const std::string& opts) {
+ std::stringstream ss(opts);
+
+ // Split on ',' character
+ while (!ss.eof()) {
+ std::string w;
+ getline(ss, w, ',');
+
+ // Trim spaces
+ w.erase(std::remove_if(w.begin(), w.end(), ::isspace), w.end());
+
+ // Extract key=value
+ auto pos = w.find('=');
+
+ // Check for bad format such as no '=' or '=' at either extremity
+ if (pos == std::string::npos || w.back() == '=' || w.front() == '=') {
+ VLOG(jdwp) << "Skipping jdwp parameters '" << opts << "', token='" << w << "'";
+ continue;
+ }
+
+ // Set
+ std::string key = w.substr(0, pos);
+ std::string value = w.substr(pos + 1);
+ put(key, value);
+ VLOG(jdwp) << "Found jdwp parameters '" << key << "'='" << value << "'";
+ }
+}
+
+void JdwpArgs::put(const std::string& key, const std::string& value) {
+ if (store.find(key) == store.end()) {
+ keys.emplace_back(key);
+ }
+
+ store[key] = value;
+}
+
+std::string JdwpArgs::join() {
+ std::string opts;
+ for (const auto& key : keys) {
+ opts += key + "=" + store[key] + ",";
+ }
+
+ // Remove the trailing comma if there is one
+ if (opts.length() >= 2) {
+ opts = opts.substr(0, opts.length() - 1);
+ }
+
+ return opts;
+}
+} // namespace adbconnection
diff --git a/adbconnection/jdwpargs.h b/adbconnection/jdwpargs.h
new file mode 100644
index 0000000..f3ade44
--- /dev/null
+++ b/adbconnection/jdwpargs.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ART_ADBCONNECTION_JDWPARGS_H_
+#define ART_ADBCONNECTION_JDWPARGS_H_
+
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+namespace adbconnection {
+
+// A key/value store which respects order of insertion when join the values.
+// This is necessary for jdwp agent parameter. e.g.: key "transport", must be
+// issued before "address", otherwise oj-libjdpw will crash.
+//
+// If a key were to be re-inserted (a.k.a overwritten), the first insertion
+// will be used for order.
+class JdwpArgs {
+ public:
+ explicit JdwpArgs(const std::string& opts);
+ ~JdwpArgs() = default;
+
+ // Add a key / value
+ void put(const std::string& key, const std::string& value);
+
+ // Concatenate all key/value into a command separated list of "key=value" entries.
+ std::string join();
+
+ private:
+ std::vector<std::string> keys;
+ std::unordered_map<std::string, std::string> store;
+};
+
+} // namespace adbconnection
+
+#endif // ART_ADBCONNECTION_JDWPARGS_H_