Skip to content

Commit

Permalink
i#2154 Android64: Fix adb argument quoting
Browse files Browse the repository at this point in the history
When building for Android with -DDR_COPY_TO_DEVICE=ON we can use CTest
to run the DynamoRIO tests on an attached Android device.

Previously this was done by adding `adb shell` to the beginning of the
test command. This works fine for many test commands but doesn't
properly preserve argument quoting so test commands which have spaces
inside an argument would get parsed incorrectly leading to test
failures.

For example if we run:
    adb shell "foo" "bar baz"
the quotes are interpreted by the host shell so the command that the
remote Android shell sees is:
    foo bar baz

The "bar baz" argument has been effectively split into two.

An easy way to solve this is to surround the test command in single
quotes:
    adb shell '"foo" "bar baz"'
so the full test command is passed to the Android shell with the
argument quotes intact:
    "foo" "bar baz"

I have changed the CMake scripts to use a wrapper script instead of
invoking adb directly (run_on_android_device.sh) which just applies this
extra layer of quoting and allows us to run tests on Android more
reliably.

Issues: #2154, #1874
  • Loading branch information
jackgallagher-arm committed Jan 22, 2025
1 parent 3521f0e commit 1c6ed4b
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
5 changes: 3 additions & 2 deletions make/utils_exposed.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,11 @@ endfunction (DynamoRIO_get_target_path_for_execution)

function (DynamoRIO_prefix_cmd_if_necessary cmd_out use_ats cmd_in)
if (ANDROID)
set(script ${PROJECT_BINARY_DIR}/tools/run_on_android_device.sh)
if (use_ats)
set(${cmd_out} "adb@shell@${cmd_in}${ARGN}" PARENT_SCOPE)
set(${cmd_out} "${script}@${cmd_in}${ARGN}" PARENT_SCOPE)
else ()
set(${cmd_out} adb shell ${cmd_in} ${ARGN} PARENT_SCOPE)
set(${cmd_out} ${script} ${cmd_in} ${ARGN} PARENT_SCOPE)
endif ()
elseif (CMAKE_CROSSCOMPILING AND DEFINED CMAKE_FIND_ROOT_PATH AND QEMU_BINARY)
if (use_ats)
Expand Down
4 changes: 4 additions & 0 deletions tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,7 @@ DR_install(FILES
"${CMAKE_CURRENT_SOURCE_DIR}/windbg-scripts/load_syms${SYMSFX}"
DESTINATION "${INSTALL_BIN}"
RENAME "load_syms${SYMSFX}.txt")

if (ANDROID)
configure_file(run_on_android_device.sh.in run_on_android_device.sh @ONLY)
endif ()
6 changes: 6 additions & 0 deletions tools/run_on_android_device.sh.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/usr/bin/sh

# Used to run a command on an Android device when building for Android
# while preserving the argument quoting used on the host.

@ADB@ shell $(printf "'%s' " "$@")

0 comments on commit 1c6ed4b

Please sign in to comment.