diff options
author | 2018-01-24 13:29:07 -0800 | |
---|---|---|
committer | 2018-01-24 18:28:13 -0800 | |
commit | 15b8113eb72b829e2026477a49e159635a48349b (patch) | |
tree | b62e2ddcc2e3fb3d1156103c2bfbef08916d3cdc /dt_fd_forward | |
parent | d3233abdf14f173bb99e3905e8543ffff845230e (diff) |
Have adbconnection handle DDMS packets if agent not loaded
If DDMS was being used on a userdebug or eng build it would cause the
libjdwp agent to be loaded on various system processes, most notably
system_server. This would cause a massive performance hit as it tries
to force system_server to jit but, due to security policies, it is
forced to interpreter instead.
To fix this we make it so that (so long as no debugger commands are
issued) DDMS commands are handled without loading the JDWP agent. When
a non-DDMS command is issued we will load and initialize the JDWP
agent which will take over handling DDMS traffic from then on.
This will ensure that in the common (for userdebug) use-case where
processes only encounter DDMS commands the process will not need to
load the full debugger.
Test: ./test.py --host -j50
Test: ./art/tools/run-libjdwp-tests.sh --mode=device
Test: Run ddms monitor on host,
adb shell stop &&
adb shell setprop dalvik.vm.jdwp-provider adbconnection
adb shell start;
Ensure that device does not start to jank
Test: Run ddms monitor on host,
adb shell stop &&
adb shell setprop dalvik.vm.jdwp-provider adbconnection
adb shell start;
Turn off ddms monitor.
Ensure that device does not start to jank
Test: Build and run
Test: use ddms monitor.
Test: Use Android Studio.
Test: Build and debug debuggable app (bandhook-kotlin)
Test: Build and debug non-debuggable app on userdebug build
(bandhook-kotlin)
Test: Debug running system process on userdebug build
(com.android.packageinstaller)
Bug: 62821960
Bug: 72456312
Bug: 72457427
Change-Id: Ib8d2af6c76bd2fac5a4b27e730695b2d016d3583
Diffstat (limited to 'dt_fd_forward')
-rw-r--r-- | dt_fd_forward/dt_fd_forward.cc | 69 | ||||
-rw-r--r-- | dt_fd_forward/dt_fd_forward.h | 4 | ||||
-rw-r--r-- | dt_fd_forward/export/fd_transport.h | 6 |
3 files changed, 52 insertions, 27 deletions
diff --git a/dt_fd_forward/dt_fd_forward.cc b/dt_fd_forward/dt_fd_forward.cc index 1cf31a75a5..116cdf84ed 100644 --- a/dt_fd_forward/dt_fd_forward.cc +++ b/dt_fd_forward/dt_fd_forward.cc @@ -276,17 +276,16 @@ static void SendAcceptMessage(int fd) { TEMP_FAILURE_RETRY(send(fd, kAcceptMessage, sizeof(kAcceptMessage), MSG_EOR)); } -IOResult FdForwardTransport::ReceiveFdsFromSocket() { +IOResult FdForwardTransport::ReceiveFdsFromSocket(bool* do_handshake) { union { cmsghdr cm; uint8_t buffer[CMSG_SPACE(sizeof(FdSet))]; } msg_union; - // We don't actually care about the data. Only FDs. We need to have an iovec anyway to tell if we - // got the values or not though. - char dummy = '\0'; + // This lets us know if we need to do a handshake or not. + char message[128]; iovec iov; - iov.iov_base = &dummy; - iov.iov_len = sizeof(dummy); + iov.iov_base = message; + iov.iov_len = sizeof(message); msghdr msg; memset(&msg, 0, sizeof(msg)); @@ -307,8 +306,22 @@ IOResult FdForwardTransport::ReceiveFdsFromSocket() { return IOResult::kError; } FdSet out_fds = FdSet::ReadData(CMSG_DATA(cmsg)); - if (out_fds.read_fd_ < 0 || out_fds.write_fd_ < 0 || out_fds.write_lock_fd_ < 0) { + bool failed = false; + if (out_fds.read_fd_ < 0 || + out_fds.write_fd_ < 0 || + out_fds.write_lock_fd_ < 0) { DT_IO_ERROR("Received fds were invalid!"); + failed = true; + } else if (strcmp(kPerformHandshakeMessage, message) == 0) { + *do_handshake = true; + } else if (strcmp(kSkipHandshakeMessage, message) == 0) { + *do_handshake = false; + } else { + DT_IO_ERROR("Unknown message sent with fds."); + failed = true; + } + + if (failed) { if (out_fds.read_fd_ >= 0) { close(out_fds.read_fd_); } @@ -346,8 +359,9 @@ jdwpTransportError FdForwardTransport::Accept() { state_cv_.wait(lk); } + bool do_handshake = false; DCHECK_NE(listen_fd_.get(), -1); - if (ReceiveFdsFromSocket() != IOResult::kOk) { + if (ReceiveFdsFromSocket(&do_handshake) != IOResult::kOk) { CHECK(ChangeState(TransportState::kOpening, TransportState::kListening)); return ERR(IO_ERROR); } @@ -355,24 +369,27 @@ jdwpTransportError FdForwardTransport::Accept() { current_seq_num_++; // Moved to the opening state. - char handshake_recv[sizeof(kJdwpHandshake)]; - memset(handshake_recv, 0, sizeof(handshake_recv)); - IOResult res = ReadFullyWithoutChecks(handshake_recv, sizeof(handshake_recv)); - if (res != IOResult::kOk || - strncmp(handshake_recv, kJdwpHandshake, sizeof(kJdwpHandshake)) != 0) { - DT_IO_ERROR("Failed to read handshake"); - CHECK(ChangeState(TransportState::kOpening, TransportState::kListening)); - CloseFdsLocked(); - // Retry. - continue; - } - res = WriteFullyWithoutChecks(kJdwpHandshake, sizeof(kJdwpHandshake)); - if (res != IOResult::kOk) { - DT_IO_ERROR("Failed to write handshake"); - CHECK(ChangeState(TransportState::kOpening, TransportState::kListening)); - CloseFdsLocked(); - // Retry. - continue; + if (do_handshake) { + // Perform the handshake + char handshake_recv[sizeof(kJdwpHandshake)]; + memset(handshake_recv, 0, sizeof(handshake_recv)); + IOResult res = ReadFullyWithoutChecks(handshake_recv, sizeof(handshake_recv)); + if (res != IOResult::kOk || + strncmp(handshake_recv, kJdwpHandshake, sizeof(kJdwpHandshake)) != 0) { + DT_IO_ERROR("Failed to read handshake"); + CHECK(ChangeState(TransportState::kOpening, TransportState::kListening)); + CloseFdsLocked(); + // Retry. + continue; + } + res = WriteFullyWithoutChecks(kJdwpHandshake, sizeof(kJdwpHandshake)); + if (res != IOResult::kOk) { + DT_IO_ERROR("Failed to write handshake"); + CHECK(ChangeState(TransportState::kOpening, TransportState::kListening)); + CloseFdsLocked(); + // Retry. + continue; + } } break; } diff --git a/dt_fd_forward/dt_fd_forward.h b/dt_fd_forward/dt_fd_forward.h index 9303c59acd..07a574bfa0 100644 --- a/dt_fd_forward/dt_fd_forward.h +++ b/dt_fd_forward/dt_fd_forward.h @@ -105,7 +105,9 @@ class FdForwardTransport : public jdwpTransportEnv { bool ChangeState(TransportState old_state, TransportState new_state); // REQUIRES(state_mutex_); - IOResult ReceiveFdsFromSocket(); + // Gets the fds from the server side. do_handshake returns whether the transport can skip the + // jdwp handshake. + IOResult ReceiveFdsFromSocket(/*out*/bool* do_handshake); IOResult WriteFully(const void* data, size_t ndata); // REQUIRES(!state_mutex_); IOResult WriteFullyWithoutChecks(const void* data, size_t ndata); // REQUIRES(state_mutex_); diff --git a/dt_fd_forward/export/fd_transport.h b/dt_fd_forward/export/fd_transport.h index 245f0c2275..144ac5c6ec 100644 --- a/dt_fd_forward/export/fd_transport.h +++ b/dt_fd_forward/export/fd_transport.h @@ -47,6 +47,12 @@ struct FdSet { } }; +// Sent with the file descriptors if the transport should not skip waiting for the handshake. +static constexpr char kPerformHandshakeMessage[] = "HANDSHAKE:REQD"; + +// Sent with the file descriptors if the transport can skip waiting for the handshake. +static constexpr char kSkipHandshakeMessage[] = "HANDSHAKE:SKIP"; + // This message is sent over the fd associated with the transport when we are listening for fds. static constexpr char kListenStartMessage[] = "dt_fd_forward:START-LISTEN"; |