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

ASan doesn't work in 32bit armeabi-v7a on pixel #840

Closed
gpx1000 opened this issue Jul 28, 2017 · 26 comments
Closed

ASan doesn't work in 32bit armeabi-v7a on pixel #840

gpx1000 opened this issue Jul 28, 2017 · 26 comments
Assignees

Comments

@gpx1000
Copy link

gpx1000 commented Jul 28, 2017

Even for a simple "Hello world" app, usage of libclang_rt.asan-arm-android.so on a Pixel running an armeabi-v7a abi app will result in:
==5512==Shadow memory range interleaves with an existing memory mapping. ASan cannot proceed correctly. ABORTING.

Same project reconfigured for 64bit mode works flawlessly.

https://github.com/google/sanitizers/wiki/AddressSanitizer seems to indicate that 32bit arm should be working. Lemme know if you need an example project.

@eugenis
Copy link
Contributor

eugenis commented Jul 28, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 28, 2017

Am using wrap.sh (see the other discussion for how am using it). The strange thing is it works in aarch64 with no other changes.

@eugenis
Copy link
Contributor

eugenis commented Jul 28, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 28, 2017

I'll have to submit it to github when I get home. Did it last night as part of the tutorial I'm putting together.

@gpx1000
Copy link
Author

gpx1000 commented Jul 29, 2017

Here's the example project.
https://github.com/gpx1000/ExampleASAN

To get it to succeed:
1.) attempt to run it on a arm64-v8a device that has su installed
a.) note that for putting us on the same page, this is a guide that works to get su installed on a production Pixel XL (my test device): https://forum.xda-developers.com/pixel-xl/how-to/guide-how-to-unlock-root-flash-pixel-xl-t3507886
2.) Run this app as is, it will auto-select arm64-v8a due to the fact the apk will be built with support for both armeabi-v7a and arm64-v8a.
3.) Note that ASan will correctly sigalert and helpfully place the cursor at the exact problem (cool huh?).

Now to get it to stop working:
1.) Clean first (so we're on a clean state).
2.) on line 4 of apps' build.gradle You should see a list of supported apis, delete arm64-v8a so it looks like this:
def SupportedABIs = ['armeabi-v7a']

3.) Now build and try to run the app, observe that ASan no longer runs the app and the app will not be able to load ASan or the hello world library.

NB. This is the project I'm using for the tutorial I'm writing. Please lemme know if you have comments or suggestions.

@eugenis
Copy link
Contributor

eugenis commented Jul 31, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 31, 2017

hmm... that's odd, Uploaded old version. will upload right version tonight (fixes the first three observations).

btw, when you're testing the debuggable attribute, are you building a debug variant and trying to run it via the debugger? (i.e. the bug with the play button in the AS toolbar). The gradle toolchain is kind enough to handle the manifest receiving a android:debuggable="true" element when a debug run is initiated).
I typically avoid putting a debuggable true/false setting in the manifests by hand due to this convenience. However, for the sake of creating an APK that can use ASan rather than only from within AS; I should add that to the build variant and have that get maintained by gradle code.

@eugenis
Copy link
Contributor

eugenis commented Jul 31, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 31, 2017

I just downloaded and validated the project on a fresh machine.
Let me take through the steps:
1.) clone into a fresh directory.
2.) the variable SupportedABIs takes a list; when more than one is set to it, then it creates a "fat apk" which supports the listed ABIs. Thus, in the initial configuration:
def SupportedABIs = ['armeabi-v7a', 'arm64-v8a']
the apk will have both libraries needed to run on both platforms. To get a 64 only build remove armeabi-v7a so the variable looks like this:
def SupportedABIs = [ 'arm64-v8a']
Creating an apk at this point will result in lib/arm64-v8a containing aarch64. Note that build-apk defaults to creating a "release" apk. (I should probably put code in pops up a message that you should turn off ASAN in the main build.gradle when doing this action):
project.ext {
useASAN = true
ndkDir = properties.getProperty('ndk.dir')
}
here useASAN needs to be false prior to any building of apks or it will just happily include the asan library (my bug).
Anyway, for the purposes of this test change the original line to read:
def SupportedABIs = ['arm64-v8a']
Now hit the debug icon which tells AS to build a debuggable apk, install it and attach the lldb server/client. You should see AS look like this:
working
Here, observe that 64 bit is working like I'm expecting it to. Now for the failure.

3.) Change the supportedABIs line to read:
def SupportedABIs = ['armeabi-v7a']
4.) hit stop button in the toolbar so we can read/write files during the next build.
5.) select the Sync Now helpful tool suggestion (this just gives the gradle a chance to rpopulate all settings and doesn't actually build anything new.
6.) Now go ahead and hit that debug button again.
nowork

I know you probably know all of the above, but thought I'd ensure everything is exactly same page. And my understanding has been known to be wrong, so I might have something really out of whack here :).

@eugenis
Copy link
Contributor

eugenis commented Jul 31, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 31, 2017

I know the feeling; I used to work at SRSLabs and spent most of my time working with the audio pipeline in audioflinger and lower. Took me a while to adjust and am now finally quite comfortable in AS; but there's a part of me that still much prefers vim/make workflow.

That is correct. Wrap.sh only technically works in O. I've been testing with PixelXL on N and above. O builds work beautifully. The other place this works is the emulator; however, even in O, I'm seeing 64bit work and 32bit fail.
I actually assumed that the requirement for wrap.sh being in O was incorrect and it was somehow in N due to it working there. I'll do some testing to see what else might not be working.

no, outputs/apk only gets populated when one selects build->buildapk. It's due to how the debugger works and the ability to do "instant run debugging" changes (i.e. change a line in java while at a breakpoint, and you can see the changes live). Due to that requirement there is no real correlation. Also, things change rapidly with the NDK team's decisions about where apks get created and simulated etc; very little is actually documented. However, to get an idea of what's getting built, the build folder contains intermediates which is going to have most of what you'd be expecting prior to it getting assembled.
The gradle task that actually does the apk magic is assemble*.
Oh and the reason why it's called app-debug.apk is "debug" is the variant-flavor name. So you'd see it called app-flavor-buildtype.apk anytime you build an apk.

@gpx1000
Copy link
Author

gpx1000 commented Jul 31, 2017

Interesting... use-after-free seems to work:

useafterfree
Code used:
int *array = new int[100];
delete [] array;
int boom = array[5];

@eugenis
Copy link
Contributor

eugenis commented Jul 31, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Jul 31, 2017

Interesting, On O it wasn't picking up the wrap.sh and I was experiencing the same thing as N. Now it makes sense (the bug from earlier of it not being in libs is what was giving false positive. Your changes look good (finding the correct path to find the library with; although I had thought that directory was already in the library search path).
I'll make a quick change to make it not create the wrap.sh of the other abis. Will report back as soon as I see it either work or fail.

p.s. it looks like this is only a solution for O and above, everything else requires an eng/dev build so can turn off dt-verify and the script we're working on. I will have to mention that in the blog when I write it.

@gpx1000
Copy link
Author

gpx1000 commented Aug 1, 2017

Okay, on O, I still have the it only works in 64 problem (can't repo the arm64 wrap.sh still existing thing).
However, when I try to debug I get the following:
07-31 18:35:01.753 18523-18523/? A/libc: CANNOT LINK EXECUTABLE "crash_dump64": "/data/app/com.gpxblog.exampleasan-zVH9t0q1rwj1-igipDhxTA==/lib/arm/libclang_rt.asan-arm-android.so" is 32-bit instead of 64-bit
07-31 18:35:01.753 18523-18523/? A/libc: Fatal signal 6 (SIGABRT), code -6 in tid 18523 (crash_dump64)

Any thoughts?

@eugenis
Copy link
Contributor

eugenis commented Aug 1, 2017 via email

@eugenis
Copy link
Contributor

eugenis commented Aug 1, 2017 via email

@eugenis
Copy link
Contributor

eugenis commented Aug 1, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Aug 1, 2017

Wow, thanks! That seems to work much better; should have thought to clear out ld_preload. I'll take a look at what I can do to clean up incremental builds and make it more bulletproof.

I think I also have a way to get the debugger to hook up. Wrap.sh starts the process from the script and the debugger seems to attach only to the wrap.sh process. I'm thinking of telling llvm to attach to the spawned process instead; but need to make that generic; anyway, work in progress on getting that one better integrated.

@gpx1000
Copy link
Author

gpx1000 commented Aug 1, 2017

So I got it working in 32 bit land. The problem was caused by "su" running which somehow caused crashdump64 to run instead of 32. This likely means that what I should do is create a separate gradle task to turn enforcing off/on and then set AS to invoke that special gradle task in the IDE.
Heck of a work around, but at least then we still get enforcing to turn off/on for security (wish didn't need it). Also get the end user workflow to be really easy.
Will work on cleaning all this up. Thanks for the help, and definitely let me know if you think of anything I can add to make this better. I think it's getting pretty usable for my larger app problems currently so that's at least a good indication that I'm on the right path here.

@eugenis
Copy link
Contributor

eugenis commented Aug 8, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Aug 8, 2017

Awesome, I was thinking about doing the same su -c echo foo that we do in the asan_device_setup script.

I am a fan of not having things show a warning or error if they're expected behavior (i.e. displaying an error in the logs seems less than ideal for keeping devs from getting confused).

I'll add the allow_user_segv_handler=1 to the options. I do like the idea of the external asan_options file existing in /data/local/tmp/asan.options.b. I'm also finding it quite useful to output the log to another temp file (some of the logs are quite verbose). This method seems to be getting quite serviceable even in my larger projects; now that I have the cleaning part fixed. Will be writing up the tutorial / blog post for using it this week and updating the github with my latest fixes.
Thanks for all the help! Please keep it coming if you think of any other ideas.

@gpx1000
Copy link
Author

gpx1000 commented Sep 13, 2017

As promised here's the tutorial put together for those that search for this:
https://medium.com/@gpx1000/oreo-ndk-secrets-7d075a9b084

@eugenis
Copy link
Contributor

eugenis commented Sep 13, 2017 via email

@gpx1000
Copy link
Author

gpx1000 commented Sep 13, 2017

Thanks! Updated to reflect that. (was correct in the github example).

@morehouse
Copy link
Contributor

@eugenis, @gpx1000: Can this be closed?

@eugenis eugenis closed this as completed Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants