Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] wrap.sh prevents debugger from attaching #933

Closed
polishcode opened this issue Mar 15, 2019 · 23 comments
Closed

[BUG] wrap.sh prevents debugger from attaching #933

polishcode opened this issue Mar 15, 2019 · 23 comments
Assignees
Labels

Comments

@polishcode
Copy link

NDK latest version (19.2.5345600)
gradle/externalNativeBuild/ndkBuild
host OS: Windows 10
compiler: clang
abi checked: arm*, x86*
stl c++_shared
NDK API level 26
device API level: 26-28 (emulator, Nexus 5X)
ASAN used with build + wrap.sh bundled

When wrap.sh is bundled with the apk, the AS debugger (native)/adb does not see any debuggable processes on the device. Therefore no debugging session can be started.

When wrap.sh is not bundled with the apk (just libclang_rt.asan-*.so), debugging session starts with no problem.

I use standard contents (generated by ndk-build) for wrap.sh:

#!/system/bin/sh
HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1
export LD_PRELOAD=$HERE/libclang_rt.asan-i686-android.so
$@

@polishcode polishcode added the bug label Mar 15, 2019
@enh
Copy link
Contributor

enh commented Mar 21, 2019

the ART folks seem to consider this working as intended, so i'm working on at least getting the documentation fixed to include the workaround. in the meantime, here's what the workaround looks like:

#!/system/bin/sh

cmd=$1
shift

os_version=$(getprop ro.build.version.sdk)

if [ "$os_version" -eq "27" ]; then
  cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
  cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
  cmd="$cmd -XjdwpProvider:adbconnection $@"
fi

exec $cmd

(you'll need to add your lines back in there, of course!)

@enh enh self-assigned this Mar 21, 2019
@polishcode
Copy link
Author

Thank you. I will try that tomorrow.

@polishcode
Copy link
Author

Ok, took me more than a day to focus on this again :)
Thank you for update: https://developer.android.com/ndk/guides/wrap-script#debugging_when_using_wrapsh

Few minor issues though after making it work for me.

  1. It would be nice to have a consistent view on using 'exec' for wrap.sh:
  1. It would be nice to mention that a change in e.g. abi requires stopping application (or uninstalling it) between app re-runs. I can only guess the reasons, but the failure to do that results in the application not being able to start (it just hangs, white screen as the UI does not render). After reinstall it is ok.

  2. After application installation from AS the Attach Debugger to Android Process windows shows dreaded "Error/Warning debug info can be unavailable...". adb usb in a cmd fixes the situation. This does not happen to me without wrap.sh.

  3. It would be great to mention that when running e.g. armeabi on a 64-bit device linker will not find getprop symbol. In runtime it will throw:

03-25 12:55:35.656 29330 29330 F DEBUG : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
03-25 12:55:35.657 29330 29330 F DEBUG : Build fingerprint: 'google/bullhead/bullhead:8.1.0/OPM7.181205.001/5080180:user/release-keys'
03-25 12:55:35.657 29330 29330 F DEBUG : Revision: 'rev_1.0'
03-25 12:55:35.657 29330 29330 F DEBUG : ABI: 'arm64'
03-25 12:55:35.657 29330 29330 F DEBUG : pid: 29327, tid: 29327, name: getprop >>> getprop <<<
03-25 12:55:35.657 29330 29330 F DEBUG : signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
03-25 12:55:35.658 29330 29330 F DEBUG : Abort message: 'CANNOT LINK EXECUTABLE "getprop": "/data/app/softax.wallet.showcase-NX3ASEF5eLcHDAqbeBZhJw==/lib/arm/libclang_rt.asan-arm-android.so" is 32-bit instead of 64-bit'
03-25 12:55:35.658 29330 29330 F DEBUG : x0 0000000000000000 x1 000000000000728f x2 0000000000000006 x3 0000000000000008
...

Nevertheless, thank you for your work.

@polishcode
Copy link
Author

  1. One or more ASAN arguments from list below prevent app from running with your fix (app freezes, UI does not render):
    abort_on_error=1,debug=true,print_stats=true,log_path=stdout
    verbosity=1,detect_stack_use_after_return=true,print_stats=true

@polishcode
Copy link
Author

5.1 Turns out option: "log_path=stdout" is not compatible with your solution.

@enh
Copy link
Contributor

enh commented Mar 26, 2019

@eugenis for 1 and 5.

for 4 instead of shelling out to getprop you probably just want to call the bionic system property APIs directly (see <sys/system_properties.h>).

@eugenis
Copy link
Collaborator

eugenis commented Mar 26, 2019

  1. It does not really matter, but it's better to use "exec" if it works. I'll update asan docs.

  2. That's a common problem with LD_LIBRARY_PATH in multiarch systems. We used a symlink trick in the previous iteration of asan-on-android:
    https://github.com/llvm-mirror/compiler-rt/blob/master/lib/asan/scripts/asan_device_setup#L429
    It's better to avoid this problem altogether by using the libc API.

  3. I'm not sure what's going on with log_path=stdout. Try log_path=stderr? Writing random stuff to stdout is known to break programs (just imagine doing this to "cat" or "grep").

@Wizermil
Copy link

Thanks @enh your script works like a charm.

Maybe the scripts in folder wrap.sh should be updated too?

#!/system/bin/sh

cmd=$1
shift

HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=abort_on_error=true,debug=true,print_stats=true,log_path=stderr,verbosity=true,detect_stack_use_after_return=true,print_stats=true
export LD_PRELOAD=$HERE/libclang_rt.asan-aarch64-android.so

os_version=$(getprop ro.build.version.sdk)
if [ "$os_version" -eq "27" ]; then
    cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
    cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
    cmd="$cmd -XjdwpProvider:adbconnection $@"
fi
exec $cmd

@HaiPhan2002
Copy link

Hello, I stuck with the wrap.sh until I read this thread. It is very useful, I understand the step what I need to do.
Unfortunately, I still can not debug my App on Pixel 3 - Android 10, Build number QQ2A.200305.002.
The Debugger still shows Could not connect to remote process. Aborting debug session.

Here is my wrap.sh

#!/system/bin/sh

cmd=$1
shift

HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1
export LD_PRELOAD=$HERE/libclang_rt.asan-aarch64-android.so

os_version=$(getprop ro.build.version.sdk)
if [ "$os_version" -eq "27" ]; then
    cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
    cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
    cmd="$cmd -XjdwpProvider:adbconnection $@"
fi
echo $cmd
exec $cmd

@DanAlbert
Copy link
Member

@enh-google anything else you want to do on the docs here?

@DanAlbert DanAlbert assigned enh-google and unassigned enh Apr 13, 2020
@DanAlbert DanAlbert added this to the unscheduled docs milestone Apr 13, 2020
@enh-google
Copy link
Collaborator

i think we should update the script https://android.googlesource.com/platform/ndk.git/+/refs/heads/master/wrap.sh/asan.sh to take care of the various API level differences automatically, but i'm not sure if you want to +2 that change without me testing it on a device :-)

@enh-google
Copy link
Collaborator

Hello, I stuck with the wrap.sh until I read this thread. It is very useful, I understand the step what I need to do.
Unfortunately, I still can not debug my App on Pixel 3 - Android 10, Build number QQ2A.200305.002.
The Debugger still shows Could not connect to remote process. Aborting debug session.

Here is my wrap.sh

#!/system/bin/sh

cmd=$1
shift

HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1
export LD_PRELOAD=$HERE/libclang_rt.asan-aarch64-android.so

os_version=$(getprop ro.build.version.sdk)
if [ "$os_version" -eq "27" ]; then
    cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
    cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
    cmd="$cmd -XjdwpProvider:adbconnection $@"
fi
echo $cmd
exec $cmd

yes, apparently new API levels also need -XjdwpOptions:suspend=n,server=y. the corrected script should look like this:

#!/system/bin/sh

cmd=$1
shift

os_version=$(getprop ro.build.version.sdk)

if [ "$os_version" -eq "27" ]; then
  cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
  cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
  cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y $@"
fi

exec $cmd

i've sent out a documentation change for review internally, so the public docs should be updated soon.

@DanAlbert DanAlbert added docs and removed bug labels Dec 17, 2020
@allight
Copy link

allight commented Feb 1, 2021

Just as an FYI with https://android-review.googlesource.com/c/platform/art/+/1526559 on 'S'+ builds (after 210202) will not require the -XjdwpOptions:suspend=n,server=y option since that will be the default value for that flag. The -XjdwpProvider:adbconnection will still be required.

@nrbrook
Copy link

nrbrook commented Mar 21, 2022

The ASan documentation still has this issue. This seems to work:

#!/system/bin/sh
HERE=$(cd "$(dirname "$0")" && pwd)

cmd=$1
shift

export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1
ASAN_LIB=$(ls "$HERE"/libclang_rt.asan-*-android.so)
if [ -f "$HERE/libc++_shared.so" ]; then
    # Workaround for https://github.com/android-ndk/ndk/issues/988.
    export LD_PRELOAD="$ASAN_LIB $HERE/libc++_shared.so"
else
    export LD_PRELOAD="$ASAN_LIB"
fi

os_version=$(getprop ro.build.version.sdk)

if [ "$os_version" -eq "27" ]; then
  cmd="$cmd -Xrunjdwp:transport=dt_android_adb,suspend=n,server=y -Xcompiler-option --debuggable $@"
elif [ "$os_version" -eq "28" ]; then
  cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y -Xcompiler-option --debuggable $@"
else
  cmd="$cmd -XjdwpProvider:adbconnection -XjdwpOptions:suspend=n,server=y $@"
fi

exec $cmd

@atsushieno
Copy link

I spent a couple of days to investigate why ASAN started not working since last time I used and scratching my head, and finally found ^. Thank you @nrbrook and I hope this gets fixed in the NDK distributions.

atsushieno added a commit to atsushieno/aap-core that referenced this issue May 25, 2022
@DanAlbert
Copy link
Member

https://android-review.googlesource.com/c/platform/ndk/+/2107184

I haven't actually gone through the test step there. Will start attempting that now. Probably overdue for updating that sample anyway...

@DanAlbert DanAlbert removed this from the unscheduled docs milestone May 25, 2022
@DanAlbert DanAlbert moved this to Triaged in NDK r26 May 25, 2022
@DanAlbert DanAlbert self-assigned this May 25, 2022
@DanAlbert DanAlbert removed the docs label May 25, 2022
@DanAlbert
Copy link
Member

Reclassifying this as a bug again since there is something for us to ship now. This bug will be sort of perpetual if the properties needed to keep jdwp working keep changing though :(

@enh-google
Copy link
Collaborator

Reclassifying this as a bug again since there is something for us to ship now. This bug will be sort of perpetual if the properties needed to keep jdwp working keep changing though :(

can we write a CTS test for this? whether we can or not, it's probably worth bringing this up with ART anyway, so they know that changes here cause real disruption for app developers...

@DanAlbert
Copy link
Member

can we write a CTS test for this?

That's what I was going to ask you :)

@enh-google
Copy link
Collaborator

can we write a CTS test for this?

That's what I was going to ask you :)

also a question for the LON folks, i think, since i don't even know why they'd ever change this stuff in the first place --- seems certain to cause breakage.

DanAlbert added a commit to DanAlbert/asan-cmake-example that referenced this issue May 25, 2022
Includes the fix from android/ndk#933. Should
probably figure out how to write a gradle task to do this automatically.
@DanAlbert DanAlbert moved this from Triaged to Merged in NDK r26 May 25, 2022
@DanAlbert
Copy link
Member

DanAlbert commented May 25, 2022

Tested the fix (my sample project has been updated) and merged into master (which will become r26). Leaving this open until I talk to the art folks though.

@cferris1000
Copy link
Collaborator

There are tests for wrap.sh test that might be expanded to cover this. Specifically found in cts/tests/tests/wrap.

@DanAlbert
Copy link
Member

Since it's our only planned release for the next year, I'm pulling this change into r25.

osspop pushed a commit to osspop/android-ndk that referenced this issue Jan 17, 2023
Bug: android/ndk#933
Test: Attached debugger to https://github.com/DanAlbert/asan-cmake-example
Change-Id: I7c98473e88e31707a50aa8702a3975c94db9ea6b
osspop pushed a commit to osspop/android-ndk that referenced this issue Jan 17, 2023
Bug: android/ndk#933
Test: Attached debugger to https://github.com/DanAlbert/asan-cmake-example
(cherry picked from commit faf3d43)
(cherry picked from commit 8b297540d0b3b621a18438d8c506421620b7e5aa)
(cherry picked from commit 26ba819)
Merged-In: I7c98473e88e31707a50aa8702a3975c94db9ea6b

Change-Id: Ide5db8dc4a24f0b398324e10e8834176ea8608b2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Merged
Development

No branches or pull requests