[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

[Jim Ingham via lldb-commits]

Thanks for the explanation!

Jim


> On Jun 15, 2018, at 2:35 AM, Pavel Labath via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> labath added a comment.
> 
> Thank you for implementing this. We've had code like this in android studio for a long time, but it's definitely better doing it in lldb instead.
> 
> I'll give some more background so that Jim and anyone else looking at this can understand what's going on. These files (I forgot what's the difference between them) contain the compiled version of Java code. As java is a "safe" language, the code should not generate any signals (modulo compiler bugs) that the runtime can't handle. The SEGV is just the implementation's way of avoiding null checks everywhere -- on the assumption that NullPointerExceptions are rare, its faster to not generate them and clean up the occasional mess in the signal handler.
> For that reason, an android app (*) will always have a SEGV handler. This handler will be invoked even for non-bening SEGVs (so simply resuming from a SEGV will never crash the app immediately). For signals the runtime can't handle it will invoke a special `art_sigsegv_fault` function, which the debugger can hook to catch "real" segfaults. Unfortunately, in the past this mechanism was unreliable (the function could end up inlined), so we have to do this dance instead. Once android versions with the fixed "fault" function become more prevalent, we can skip this and just automatically reinject all SIGSEGVs. This is particularly important as each new version of android finds new creative ways to "optimize" things via SIGSEGVs, and going things this way means constantly playing catch-up.
> 
> So much for background. I think Jim's suggestion on having all of this this controllable by a setting makes sense, and it would be consistent with how we handle other kinds of "magical" under-the-hood stops (target.process.stop-on-sharedlibrary-events). I'm not sure how much use would it get, but I can imagine it being useful for debugging the segv handling code itself. I'm a bit sad that we now have two plugins with the `OverrideStopInfo` functionality, but I can't think of any better way of arranging things right now.
> 
> (*) This means "real" GUI apps. command line executables will not have the android runtime inside them, nor the special segv handler, but that means they will not contain any "dex" files either.
> 
> 
> 
> ================
> Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:399
> +// Define a SIGSEGV that doesn't require any headers
> +#define ANDROID_SIGSEGV 11
> +
> ----------------
> I'm not sure if SEGV is one of them, but numbers of some signals vary between architectures. You should be able to get the real value via process->GetUnixSignals()
> 
> 
> ================
> Comment at: source/Plugins/Platform/Android/PlatformAndroid.cpp:424
> +  // We are lookking for .dex, .odex, and .oat files.
> +  if (ext_ref.endswith("dex") || ext_ref.endswith("oat")) {
> +    // We have a SIGSEGV we need to mute
> ----------------
> jingham wrote:
>> Given that this is a pretty big behavior change, I would exactly match on the three extensions rather than use endswith, so it only affects the file types you care about.
> Agreed, matching on the exact extension looks safer and more obvious. The most llvm-y way of writing that would be `StringSwitch<bool>(ext.GetStringRef()).Cases("dex", "oat", "odex", true).Default(false)`
> 
> 
> https://reviews.llvm.org/D48177
> 
> 
>