From 03bee486598d54d78837459f76185a48bca27a35 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 20 Apr 2020 19:21:41 -0700 Subject: [PATCH 1/5] adbd: add unit tests to coverage report. Test: ./coverage.sh Change-Id: I268367b857efc2125c6714a1d0b0e874fa8d143d --- adb/coverage.sh | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/adb/coverage.sh b/adb/coverage.sh index 50ccdf5d6..bf46f7981 100755 --- a/adb/coverage.sh +++ b/adb/coverage.sh @@ -15,7 +15,25 @@ set -euxo pipefail +ADB_TESTS="adbd_test adb_crypto_test adb_pairing_auth_test adb_pairing_connection_test adb_tls_connection_test" + +TRACEDIR=`mktemp -d` + adb root + +### Run the adb unit tests and fetch traces from them. +mkdir "$TRACEDIR"/test_traces +adb shell rm -rf /data/local/tmp/adb_coverage +adb shell mkdir /data/local/tmp/adb_coverage + +for TEST in $ADB_TESTS; do + adb shell LLVM_PROFILE_FILE=/data/local/tmp/adb_coverage/$TEST.profraw /data/nativetest64/$TEST/$TEST + adb pull /data/local/tmp/adb_coverage/$TEST.profraw "$TRACEDIR"/test_traces/ +done + +adb pull /data/local/tmp/adb_coverage "$TRACEDIR"/test_traces + +### Run test_device.py, and fetch traces from adbd itself. adb shell logcat -c -G128M adb shell setprop persist.adb.trace_mask 1 adb shell killall adbd @@ -23,7 +41,7 @@ adb shell killall adbd # TODO: Add `adb transport-id` and wait-for-offline on it. sleep 5 -adb wait-for-device shell rm "/data/misc/trace/*" +adb wait-for-device shell rm -rf "/data/misc/trace/*" /data/local/tmp/adb_coverage/ ./test_device.py @@ -33,11 +51,9 @@ adb shell killall -37 adbd echo Waiting for adbd to finish dumping traces sleep 5 -TRACEDIR=`mktemp -d` adb pull /data/misc/trace "$TRACEDIR"/ echo Pulled traces to $TRACEDIR - # Identify which of the trace files are actually adbd, in case something else exited simultaneously. ADBD_PIDS=$(adb shell "logcat -d -s adbd --format=process | grep 'adbd started' | cut -c 3-7 | tr -d ' ' | sort | uniq") mkdir "$TRACEDIR"/adbd_traces @@ -48,16 +64,25 @@ IFS=$'\n' for PID in $ADBD_PIDS; do cp "$TRACEDIR"/trace/clang-$PID-*.profraw "$TRACEDIR"/adbd_traces 2>/dev/null || true done +unset IFS -llvm-profdata merge --output="$TRACEDIR"/adbd.profdata "$TRACEDIR"/adbd_traces/* +ADB_TEST_BINARIES="" +for TEST in $ADB_TESTS; do + ADB_TEST_BINARIES="--object=$ANDROID_PRODUCT_OUT/data/nativetest64/$TEST/$TEST $ADB_TEST_BINARIES" +done + +### Merge the traces and generate a report. +llvm-profdata merge --output="$TRACEDIR"/adbd.profdata "$TRACEDIR"/adbd_traces/* "$TRACEDIR"/test_traces/* cd $ANDROID_BUILD_TOP llvm-cov report --instr-profile="$TRACEDIR"/adbd.profdata \ $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ --show-region-summary=false \ - /proc/self/cwd/system/core/adb + /proc/self/cwd/system/core/adb \ + $ADB_TEST_BINARIES llvm-cov show --instr-profile="$TRACEDIR"/adbd.profdata \ $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ --format=html \ - /proc/self/cwd/system/core/adb > $TRACEDIR/report.html + /proc/self/cwd/system/core/adb \ + $ADB_TEST_BINARIES > $TRACEDIR/report.html From e3c62cc549aa2d20c634b98716fcd3890ac6dbdb Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 21 Apr 2020 16:24:04 -0700 Subject: [PATCH 2/5] adb: allow wait-for-disconnect to match offline for TCP devices. This fixes a bug in adb root/unroot where we always fail because we're waiting for a TCP device to disappear. Test: test_device.py over TCP Change-Id: I7e4b6fdaa1070cee1f9b471de46ae00bf89b3089 --- adb/services.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/adb/services.cpp b/adb/services.cpp index d87948c84..19a9030f2 100644 --- a/adb/services.cpp +++ b/adb/services.cpp @@ -202,11 +202,22 @@ static void wait_service(unique_fd fd, std::string serial, TransportId transport transport_id, &is_ambiguous, &error); for (const auto& state : states) { - // wait-for-disconnect uses kCsOffline, we don't actually want to wait for 'offline'. - if ((t == nullptr && state == kCsOffline) || (t != nullptr && state == kCsAny) || - (t != nullptr && state == t->GetConnectionState())) { - SendOkay(fd); - return; + if (state == kCsOffline) { + // Special case for wait-for-disconnect: + // We want to wait for USB devices to completely disappear, but TCP devices can + // go into the offline state, since we automatically reconnect. + if (!t) { + SendOkay(fd); + return; + } else if (!t->GetUsbHandle()) { + SendOkay(fd); + return; + } + } else { + if (t && (state == kCsAny || state == t->GetConnectionState())) { + SendOkay(fd); + return; + } } } From 79d4f148d6d92284ce75473ae713a6ca410eba17 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 21 Apr 2020 14:22:50 -0700 Subject: [PATCH 3/5] adbd: test TCP in coverage. Test: ./coverage.sh Change-Id: I82a0d54fcc1e19585a27c2d049be40289bf15f60 --- adb/coverage.sh | 61 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/adb/coverage.sh b/adb/coverage.sh index bf46f7981..cde9b6072 100755 --- a/adb/coverage.sh +++ b/adb/coverage.sh @@ -16,10 +16,42 @@ set -euxo pipefail ADB_TESTS="adbd_test adb_crypto_test adb_pairing_auth_test adb_pairing_connection_test adb_tls_connection_test" - TRACEDIR=`mktemp -d` +### Make sure we can connect to the device. + +# Get the device's wlan0 address. +IP_ADDR=$(adb shell ip route get 0.0.0.0 oif wlan0 | sed -En -e 's/.*src (\S+)\s.*/\1/p') +REMOTE_PORT=5555 +REMOTE=$IP_ADDR:$REMOTE_PORT +LOCAL_SERIAL=$(adb shell getprop ro.serialno) + +# Check that we can connect to it. +adb disconnect +adb tcpip $REMOTE_PORT + +# TODO: Add `adb transport-id` and wait-for-offline on it. +sleep 5 + +adb connect $REMOTE + +REMOTE_FETCHED_SERIAL=$(adb -s $REMOTE shell getprop ro.serialno) + +if [[ "$LOCAL_SERIAL" != "$REMOTE_FETCHED_SERIAL" ]]; then + echo "Mismatch: local serial = $LOCAL_SERIAL, remote serial = $REMOTE_FETCHED_SERIAL" + exit 1 +fi + +# Back to USB, and make sure adbd is root. +adb disconnect $REMOTE + adb root +adb wait-for-device usb + +# TODO: Add `adb transport-id` and wait-for-offline on it. +sleep 5 + +adb wait-for-device ### Run the adb unit tests and fetch traces from them. mkdir "$TRACEDIR"/test_traces @@ -33,24 +65,47 @@ done adb pull /data/local/tmp/adb_coverage "$TRACEDIR"/test_traces -### Run test_device.py, and fetch traces from adbd itself. +# Clear logcat and increase the buffer to something ridiculous so we can fetch the pids of adbd later. adb shell logcat -c -G128M + +# Turn on extremely verbose logging so as to not count debug logging against us. adb shell setprop persist.adb.trace_mask 1 + +### Run test_device.py over USB. adb shell killall adbd # TODO: Add `adb transport-id` and wait-for-offline on it. sleep 5 adb wait-for-device shell rm -rf "/data/misc/trace/*" /data/local/tmp/adb_coverage/ - ./test_device.py +# Do a usb reset to exercise the disconnect code. +adb_usbreset +adb wait-for-device + # Dump traces from the currently running adbd. adb shell killall -37 adbd echo Waiting for adbd to finish dumping traces sleep 5 +# Restart adbd in tcp mode. +adb tcpip $REMOTE_PORT +sleep 5 +adb connect $REMOTE +adb -s $REMOTE wait-for-device + +# Run test_device.py again. +ANDROID_SERIAL=$REMOTE ./test_device.py + +# Dump traces again. +adb disconnect $REMOTE +adb shell killall -37 adbd + +echo Waiting for adbd to finish dumping traces +sleep 5 + adb pull /data/misc/trace "$TRACEDIR"/ echo Pulled traces to $TRACEDIR From f099105f07bb3fe2bae6655d24bc4007bef71300 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 21 Apr 2020 16:18:18 -0700 Subject: [PATCH 4/5] adb: refactor and relocate coverage script. Test: ./coverage/gen_coverage.sh Change-Id: Iaff2b1577e32ba4ec8647a418a2ff4a1e9a0835d --- adb/coverage/.gitignore | 2 + adb/{coverage.sh => coverage/gen_coverage.sh} | 43 +++---------------- adb/coverage/include.sh | 5 +++ adb/coverage/report.sh | 22 ++++++++++ adb/coverage/show.sh | 12 ++++++ 5 files changed, 48 insertions(+), 36 deletions(-) create mode 100644 adb/coverage/.gitignore rename adb/{coverage.sh => coverage/gen_coverage.sh} (65%) create mode 100644 adb/coverage/include.sh create mode 100755 adb/coverage/report.sh create mode 100755 adb/coverage/show.sh diff --git a/adb/coverage/.gitignore b/adb/coverage/.gitignore new file mode 100644 index 000000000..b6a258204 --- /dev/null +++ b/adb/coverage/.gitignore @@ -0,0 +1,2 @@ +/adbd.profdata +/report diff --git a/adb/coverage.sh b/adb/coverage/gen_coverage.sh similarity index 65% rename from adb/coverage.sh rename to adb/coverage/gen_coverage.sh index cde9b6072..cced62a29 100755 --- a/adb/coverage.sh +++ b/adb/coverage/gen_coverage.sh @@ -1,21 +1,10 @@ #!/bin/bash -# Copyright (C) 2020 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. set -euxo pipefail -ADB_TESTS="adbd_test adb_crypto_test adb_pairing_auth_test adb_pairing_connection_test adb_tls_connection_test" +OUTPUT_DIR=$(dirname "$0") +. "$OUTPUT_DIR"/include.sh + TRACEDIR=`mktemp -d` ### Make sure we can connect to the device. @@ -78,7 +67,7 @@ adb shell killall adbd sleep 5 adb wait-for-device shell rm -rf "/data/misc/trace/*" /data/local/tmp/adb_coverage/ -./test_device.py +"$OUTPUT_DIR"/../test_device.py # Do a usb reset to exercise the disconnect code. adb_usbreset @@ -97,7 +86,7 @@ adb connect $REMOTE adb -s $REMOTE wait-for-device # Run test_device.py again. -ANDROID_SERIAL=$REMOTE ./test_device.py +ANDROID_SERIAL=$REMOTE "$OUTPUT_DIR"/../test_device.py # Dump traces again. adb disconnect $REMOTE @@ -121,23 +110,5 @@ for PID in $ADBD_PIDS; do done unset IFS -ADB_TEST_BINARIES="" -for TEST in $ADB_TESTS; do - ADB_TEST_BINARIES="--object=$ANDROID_PRODUCT_OUT/data/nativetest64/$TEST/$TEST $ADB_TEST_BINARIES" -done - -### Merge the traces and generate a report. -llvm-profdata merge --output="$TRACEDIR"/adbd.profdata "$TRACEDIR"/adbd_traces/* "$TRACEDIR"/test_traces/* - -cd $ANDROID_BUILD_TOP -llvm-cov report --instr-profile="$TRACEDIR"/adbd.profdata \ - $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ - --show-region-summary=false \ - /proc/self/cwd/system/core/adb \ - $ADB_TEST_BINARIES - -llvm-cov show --instr-profile="$TRACEDIR"/adbd.profdata \ - $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ - --format=html \ - /proc/self/cwd/system/core/adb \ - $ADB_TEST_BINARIES > $TRACEDIR/report.html +### Merge the traces. +llvm-profdata merge --output="$OUTPUT_DIR"/adbd.profdata "$TRACEDIR"/adbd_traces/* "$TRACEDIR"/test_traces/* diff --git a/adb/coverage/include.sh b/adb/coverage/include.sh new file mode 100644 index 000000000..45ebc3475 --- /dev/null +++ b/adb/coverage/include.sh @@ -0,0 +1,5 @@ +ADB_TESTS="adbd_test adb_crypto_test adb_pairing_auth_test adb_pairing_connection_test adb_tls_connection_test" +ADB_TEST_BINARIES="" +for TEST in $ADB_TESTS; do + ADB_TEST_BINARIES="--object=$ANDROID_PRODUCT_OUT/data/nativetest64/$TEST/$TEST $ADB_TEST_BINARIES" +done diff --git a/adb/coverage/report.sh b/adb/coverage/report.sh new file mode 100755 index 000000000..257310c6a --- /dev/null +++ b/adb/coverage/report.sh @@ -0,0 +1,22 @@ +#!/bin/bash + +set -euxo pipefail + +OUTPUT_DIR=$(realpath $(dirname "$0")) +. "$OUTPUT_DIR"/include.sh + +rm -rf "$OUTPUT_DIR"/report + +cd $ANDROID_BUILD_TOP +llvm-cov show --instr-profile="$OUTPUT_DIR"/adbd.profdata \ + $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ + /proc/self/cwd/system/core/adb \ + $ADB_TEST_BINARIES \ + --show-region-summary=false \ + --format=html -o "$OUTPUT_DIR"/report + +llvm-cov report --instr-profile="$OUTPUT_DIR"/adbd.profdata \ + $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ + /proc/self/cwd/system/core/adb \ + $ADB_TEST_BINARIES \ + --show-region-summary=false diff --git a/adb/coverage/show.sh b/adb/coverage/show.sh new file mode 100755 index 000000000..7ea7949ef --- /dev/null +++ b/adb/coverage/show.sh @@ -0,0 +1,12 @@ +#!/bin/bash + +set -euxo pipefail + +OUTPUT_DIR=$(realpath $(dirname "$0")) +. "$OUTPUT_DIR"/include.sh + +cd $ANDROID_BUILD_TOP +llvm-cov show --instr-profile="$OUTPUT_DIR"/adbd.profdata \ + $ANDROID_PRODUCT_OUT/apex/com.android.adbd/bin/adbd \ + /proc/self/cwd/system/core/adb \ + $ADB_TEST_BINARIES From 073c8d2394309a40a398963df307551ba47ede45 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 20 Apr 2020 19:22:05 -0700 Subject: [PATCH 5/5] adbd: improve coverage by compiling less code. Test: ./coverage/gen_coverage.sh && ./coverage/report.sh Change-Id: I26f823e2587435b7ec2c6ca05955adef5cfc23a0 --- adb/Android.bp | 19 +++++++++++++------ adb/adb_trace.cpp | 11 +++-------- adb/fdevent/fdevent.cpp | 3 +++ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/adb/Android.bp b/adb/Android.bp index 19e921f8c..8779c0ab2 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -165,7 +165,6 @@ libadb_srcs = [ "adb_unique_fd.cpp", "adb_utils.cpp", "fdevent/fdevent.cpp", - "fdevent/fdevent_poll.cpp", "services.cpp", "sockets.cpp", "socket_spec.cpp", @@ -176,6 +175,17 @@ libadb_srcs = [ "types.cpp", ] +libadb_darwin_srcs = [ + "fdevent/fdevent_poll.cpp", +] + +libadb_windows_srcs = [ + "fdevent/fdevent_poll.cpp", + "sysdeps_win32.cpp", + "sysdeps/win32/errno.cpp", + "sysdeps/win32/stat.cpp", +] + libadb_posix_srcs = [ "sysdeps_unix.cpp", "sysdeps/posix/network.cpp", @@ -219,7 +229,7 @@ cc_library_host_static { srcs: ["client/usb_linux.cpp"] + libadb_linux_srcs, }, darwin: { - srcs: ["client/usb_osx.cpp"], + srcs: ["client/usb_osx.cpp"] + libadb_darwin_srcs, }, not_windows: { srcs: libadb_posix_srcs, @@ -228,10 +238,7 @@ cc_library_host_static { enabled: true, srcs: [ "client/usb_windows.cpp", - "sysdeps_win32.cpp", - "sysdeps/win32/errno.cpp", - "sysdeps/win32/stat.cpp", - ], + ] + libadb_windows_srcs, shared_libs: ["AdbWinApi"], }, }, diff --git a/adb/adb_trace.cpp b/adb/adb_trace.cpp index cea24fe2f..c579dde92 100644 --- a/adb/adb_trace.cpp +++ b/adb/adb_trace.cpp @@ -89,18 +89,13 @@ void start_device_log(void) { int adb_trace_mask; -std::string get_trace_setting_from_env() { +std::string get_trace_setting() { +#if ADB_HOST const char* setting = getenv("ADB_TRACE"); if (setting == nullptr) { setting = ""; } - - return std::string(setting); -} - -std::string get_trace_setting() { -#if ADB_HOST - return get_trace_setting_from_env(); + return setting; #else return android::base::GetProperty("persist.adb.trace_mask", ""); #endif diff --git a/adb/fdevent/fdevent.cpp b/adb/fdevent/fdevent.cpp index fd550200f..70cb9b3a6 100644 --- a/adb/fdevent/fdevent.cpp +++ b/adb/fdevent/fdevent.cpp @@ -27,7 +27,10 @@ #include "adb_utils.h" #include "fdevent.h" #include "fdevent_epoll.h" + +#if !defined(__linux__) #include "fdevent_poll.h" +#endif using namespace std::chrono_literals; using std::chrono::duration_cast;