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

std::thread crash on Android 4.x with overloaded new/delete operator #287

Closed
gdhadiwal opened this issue Jan 25, 2017 · 14 comments
Closed
Assignees

Comments

@gdhadiwal
Copy link

gdhadiwal commented Jan 25, 2017

Description

App crashes with the usage of std::thread (libc++_shared.so) when used in conjunction with overloaded new/delete operator. Sample gradle project attached for reference.
Note that the crash is only observer on Android 4.x devices.

Environment Details

  • NDK Version: r12b / r13b.
  • Host OS: Mac
  • Compiler: Clang
  • ABI: armeabi, armeabi-v7a
  • STL: libc++ (shared)
  • NDK API level:
  • Device API level: 4.x

AndroidCrash.zip

@DanAlbert DanAlbert self-assigned this Jan 27, 2017
@gdhadiwal
Copy link
Author

@DanAlbert : Any updates?

@falken42
Copy link

falken42 commented Feb 9, 2017

No idea if this is the actual problem or not, but your example code is missing the overloaded operator delete[] taking std::nothrow_t as a parameter. If libc++'s implementation of std::thread is using that operator, then that would most definitely cause free() to crash.

@gdhadiwal
Copy link
Author

gdhadiwal commented Feb 15, 2017

Its not an issue with missing operator delete[](void* ptr, const std::nothrow_t&).
It is using operator delete(void* ptr) but not the overloaded one.
It only crashes on Android 4.x.
I've attached updated CrashTest sample gradle project.

Crash logs from few 4.x devices (Full Logs Attached)

//////////////////////////////////////////////////////////////////////////////////////////
// LG-D950 Android 4.4.2, API 19
//////////////////////////////////////////////////////////////////////////////////////////
backtrace:
#00 pc 00011b8a /system/lib/libc.so (dlfree+569)
#1 pc 0000d073 /system/lib/libc.so (free+10)
#2 pc 00065567 /data/app-lib/com.ea.crashtest/libc++_shared.so (operator delete(void*)+8)
#3 pc 0006f74f /data/app-lib/com.ea.crashtest/libc++_shared.so (std::__ndk1::__thread_specific_ptrstd::__ndk1::__thread_struct::__at_thread_exit(void*)+12)
#4 pc 0000e368 /system/lib/libc.so (pthread_exit+320)
#5 pc 0000dc24 /system/lib/libc.so (pthread_create+160)

//////////////////////////////////////////////////////////////////////////////////////////
// Samsung SGH-I747 Android 4.4.2, API 19
//////////////////////////////////////////////////////////////////////////////////////////
backtrace:
#00 pc 000114ac /system/lib/libc.so (dlfree+223)
#1 pc 0000dd13 /system/lib/libc.so (free+10)
#2 pc 0000d750 /system/lib/libc.so
#3 pc 0000ef50 /system/lib/libc.so (pthread_exit+80)
#4 pc 0000d410 /system/lib/libc.so (pthread_create+240)

//////////////////////////////////////////////////////////////////////////////////////////
// Samsung I9100 Android 4.1.2, API 16
//////////////////////////////////////////////////////////////////////////////////////////
backtrace:
#00 pc 00013a08 /system/lib/libc.so
#1 pc 00015de5 /system/lib/libc.so (dlfree+1628)
#2 pc 00016f93 /system/lib/libc.so (free+10)
#3 pc 00012c60 /system/lib/libc.so (pthread_exit+320)
#4 pc 00012558 /system/lib/libc.so (pthread_create+172)

//////////////////////////////////////////////////////////////////////////////////////////
// HTC Sensation 4G Android 4.0.3, API 15
//////////////////////////////////////////////////////////////////////////////////////////
backtrace:
#00 pc 00017db4 /system/lib/libc.so
#1 pc 00013d16 /system/lib/libc.so
#2 pc 00016050 /system/lib/libc.so (dlfree)
#3 pc 000166c8 /system/lib/libc.so (free)
#4 pc 00065566 /data/data/com.ea.crashtest/lib/libc++_shared.so (_ZdlPv)
#5 pc 0006f74e /data/data/com.ea.crashtest/lib/libc++_shared.so (_ZNSt6__ndk121__thread_specific_ptrINS_15__thread_structEE16__at_thread_exitEPv)
#6 pc 00013230 /system/lib/libc.so (pthread_exit)
#7 pc 00012ef4 /system/lib/libc.so (pthread_create)

CrashTest.zip
Crash Logs.txt

@gdhadiwal
Copy link
Author

@DanAlbert : Do you require more info?
It would be a great assistance if you could provide some pointers to try / experiment.

@DanAlbert
Copy link
Member

No, I haven't had time to look at this and probably won't any time soon.

@gdhadiwal
Copy link
Author

@DanAlbert : Wondering if you can spare some time on this.
This is blocking our ability to overloaded new/delete operator.

@DanAlbert
Copy link
Member

I'll see if I can spare some time while waiting on tests/builds, but I have r14b and r15 beta 1 to worry about atm. I'll be taking a sweep through our open bugs between beta 1 and beta 2 (next month, unless I need to delay beta 1).

(even if I did this right now, the fix wouldn't be in a stable release until June, which is the same if I were to look at it in April)

@gdhadiwal
Copy link
Author

@DanAlbert : Any updates?
Could you please consider this on priority.

@DanAlbert
Copy link
Member

I'm looking at this now.

@DanAlbert
Copy link
Member

Checked another couple versions to narrow down when this starts working. Fails on KitKat, passes on L MR1.

@DanAlbert
Copy link
Member

This is looking like a platform bug in the linker. My current theory is that you're getting the replacement new/delete in your libmain.so, but not in your libc++_shared.so. A new in an inline function in a libc++ header then gets deleted in libc++_shared.so function, causing the offset address to be passed to free (or vice versa, where a non-offset address is passed to delete).

This sounds quite a bit like the bugs we were fixing in the linker for L, and the fact that switching to libc++_static makes the problem go away reinforces that hypothesis. I haven't been able to come up with a good test case to confirm this though.

I'll dig around in the linker history later to see if I can find the bug describing this sort of problem.

Unfortunately, if I'm right about the cause, there's not really anything we can do to fix this. You'll either have to switch to libc++_static or avoid replacing new/delete on pre-L. I don't know what your app is like, but the optimal model for native code in an Android app is to have a single shared library in your app (lets ld eliminate the most code and avoids most of the strange device issues like this one), so unless you have some third-party middleware that's only provided as a shared library, that might be worth looking in to.

@gdhadiwal
Copy link
Author

Thanks for the update. This does sound like a linker bug.

@DanAlbert
Copy link
Member

https://android-review.googlesource.com/c/105825
https://android-review.googlesource.com/c/111627

The former, I believe. This isn't possible to fix or workaround as far as I know, so closing :(

@DanAlbert
Copy link
Member

(no workaround that doesn't involve libc++_static, anyway)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants