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

"pure virtual method called" when using std::thread with clang in NDK r12b #151

Closed
petya2164 opened this issue Jul 7, 2016 · 11 comments
Closed

Comments

@petya2164
Copy link

I have a runtime issue with the following std::thread example (thread.cpp):

#include <iostream>
#include <thread>

void function()
{
  std::cerr << "thread\n";
}

int main (int argc, char * argv [])
{
  std::cerr << "starting\n";
  std::thread thr(function);
  std::cerr << "joining\n";
  thr.join();
  std::cerr << "done\n";
  return 0;
}

If I compile the example with clang in NDK r12b (using a standalone toolchain):
arm-linux-androideabi-clang++ -std=c++11 -march=armv7-a thread.cpp -o thread
I get the following runtime failure on various Android devices (I tried Android 4.0, 4.4 and 6.0):

starting
pure virtual method called
joining
terminate called without an active exception
Segmentation fault

If I compile with g++ (from the same standalone toolchain):
arm-linux-androideabi-g++ -std=c++11 -march=armv7-a thread.cpp -o thread
Then I get the expected output on all devices:

starting
joining
thread
done

Clang had a similar issue in the past: https://llvm.org/bugs/show_bug.cgi?id=12730
An incorrect g++ configuration led to the same issue on ARM: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62100
Any ideas for a fix?

@DanAlbert
Copy link
Member

Which STL?

@petya2164
Copy link
Author

I used the default STL, I just checked that the default is gnustl in make_standalone_toolchain.py.
The toolchain was created like this:
python make_standalone_toolchain.py --arch arm --api 9

Is it recommended to use libc++ instead for clang?
Is it supposed to be feature-complete and stable on Android?

@DanAlbert
Copy link
Member

gnustl is a safer bet than the libc++ we have in the NDK right now. libc++ is getting closer, but we're not really to the point that I'm happy recommending it yet. It's a useful data point for reproducing the error though, and we've definitely seen cases where clang and gnustl don't get along in some of the C++11 parts of the library (typically atomic and type_traits, but I think we've seen thread issues too).

https://llvm.org/bugs/show_bug.cgi?id=12730#c10 seems to be the really interesting piece, but according to the following messages on that bug it should be fixed...

I'll do a bit of digging to make sure all those pieces are on for Android.

@DanAlbert DanAlbert self-assigned this Jul 7, 2016
@DanAlbert
Copy link
Member

Interestingly I don't see this problem if I use ndk-build, only via standalone toolchains.

@DanAlbert
Copy link
Member

Found the issue, and it's actually one I've reported before (internally).

The problem is that Clang doesn't search subarchitecture directories when linking libraries. Instead of passing -L toolchain/arm-linux-androideabi/lib/armv7-a to the linker, it passes toolchain/arm-linux-androideabi/lib (the arm5 directory), which results in the same bug you linked in your initial report (good find, btw).

Since using ndk-build doesn't have the toolchain in the standard GCC cross compiler layout, the default directories aren't used and the library is just linked manually.

Clang was recently fixed upstream, so once we can update Clang this will be resolved.

Unfortunately, Clang has moved to new enough Windows APIs that we can't actually build Clang until we sort some things out in our build infrastructure, so I don't know how soon that will be.

To workaround this until then you can pass -L toolchain/arm-linux-androideabi/lib/armv7-a manually when building for armv7 (no other ABI will have this problem). It's a pain, but it should at least get you moving until we get Clang updated again.

@petya2164
Copy link
Author

Thank you very much for finding the root cause of the issue!
I can confirm that passing -L toolchain/arm-linux-androideabi/lib/armv7-a solves the problem. Good to hear that clang was already patched and the fix will be included in the next update.

In the meantime, I also encountered a linker issue with this std::future example:

#include <iostream>
#include <future>

int main()
{
  std::future<int> f1 = std::async([](){ return 42; });
  f1.wait();
  std::cout << "Magic number is: " << f1.get() << std::endl;
  return 0;
}

And the same workaround fixes that problem as well!
So this can be a remedy for other STL-related issues.

@stephenhines
Copy link
Collaborator

Just to be pedantic, the clang fix is not in any scheduled NDK update yet. We hope to get it into a future release, but Windows issues are blocking that.

@petya2164
Copy link
Author

This issue is still present with NDK r13b (considering the standalone toolchain). Do you have any updates about the Windows issues that you mentioned above?

@DanAlbert
Copy link
Member

The Windows toolchain issues are fixed and we have an updated Clang in r14. @stephenhines should be able to confirm that we have this fix (I think we do, but I'm not 100% certain).

This is available in the canary builds if you're feeling brave (although the beta should be published in a few hours, or so I'm told): https://android.googlesource.com/platform/ndk/+/master/docs/ContinuousBuilds.md

@stephenhines
Copy link
Collaborator

This is likely fixed, but awaiting verification.

@petya2164
Copy link
Author

I can confirm that this issue is fixed in r14. It is now safe to use clang as a standalone toolchain. I will close this issue.

miodragdinic pushed a commit to MIPS/ndk that referenced this issue Apr 17, 2018
libandroid_support's definition of the byte-order for doubles on
armeabi was wrongly big endian rather than following the byte order of
the ABI. This was due to a missing check for __ARM_EABI__ (a bug in
the upstream FreeBSD libm source that bionic did not have because it
has been updated since it was fixed).

I haven't pulled the full update of math_private.h since it also
needs a lot of new macros for complex math support. I'll be cleaning
up libandroid_support more thoroughly soon anyway.

Note that this bug could surface even for arm7 when using a
standalone toolchain because we still have the Clang bug where it
doesn't search architecture specific lib directories, meaning that
arm5 gets used instead of arm7 (http://b/26180518 and
android/ndk#151).

Test: ./run_tests.py --filter log2
Bug: android/ndk#204
Change-Id: I2caaeac3afe0830008b865bc124f48200ac5f4b6
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