Clang LTO + thread_local is broken

[tavianator]

Some libraries/executables I've built with clang and -flto give the following error:

CANNOT LINK EXECUTABLE: cannot locate symbol "" referenced by "./executable"...
page record for 0xb6ade01c was not found (block_size=16)

Or for shared libs:

dlopen failed: cannot locate symbol "" referenced by "/data/app/...so"...

objdump -tT doesn't show anything strange that I can see, so I'm not sure why the runtime linker thinks it needs an empty-named symbol.

[DanAlbert]

I doubt we can do anything about this without a repro case.

[tavianator]

Just figured out a reproducer:

$ cat foo.cpp
#include <iostream>

struct Foo {
  ~Foo() {
    std::cout << "~Foo()\n";
  }
};

int main() {
  thread_local Foo foo;
  return 0;
}
$ arm-linux-androideabi-clang++ -std=c++11 -O3 -pie -static-libstdc++ foo.cpp
$ adb push a.out /data/local/tmp/ && adb shell /data/local/tmp/a.out
[100%] /data/local/tmp/a.out
~Foo()
$ arm-linux-androideabi-clang++ -std=c++11 -O3 -pie -static-libstdc++ -flto foo.cpp
$ adb push a.out /data/local/tmp/ && adb shell /data/local/tmp/a.out
[100%] /data/local/tmp/a.out
CANNOT LINK EXECUTABLE: cannot locate symbol "" referenced by "/data/local/tmp/a.out"...

[DanAlbert]

Great, thanks.

[stephenhines]

I don't think clang emutls support has shipped in an NDK yet.

[tavianator]

@stephenhines Hrm? The NDK's libgcc.a seems to provide __emutls_get_address() and friends. Everything works without -flto.

[DanAlbert]

So, the test case that you have is something we can't support right now anyway. I think there's still a bug here not related to that, but just FYI that non-trivial thread_local destructors don't work yet. Full explanation at the bottom.

@dimitry- thinks this is probably related to https://llvm.org/bugs/show_bug.cgi?id=24135

I don't think clang emutls support has shipped in an NDK yet.

It has (afaik), but actually the repro case in question is a different story. Strictly speaking we support __thread. We can support trivial cases of thread_local, but thread_local with non-trivial destructors requires run time support that wasn't available until M (and I think the NDK's current libc++abi doesn't have the support either). There was actually a patch recently made to libc++abi adding support for this on platforms that don't have native support, so once that lands and we pick it up we could actually support that on older platforms.

[tavianator]

@dimitry- thinks this is probably related to https://llvm.org/bugs/show_bug.cgi?id=24135

That's my guess too, I was just gonna post it.

I don't think clang emutls support has shipped in an NDK yet.

It has (afaik), but actually the repro case in question is a different story. Strictly speaking we support __thread. We can support trivial cases of thread_local, but thread_local with non-trivial destructors requires run time support that wasn't available until M (and I think the NDK's current libc++abi doesn't have the support either).

Yep, the toolchain I was using for the repro is actually clang with gnustl. gnustl provides a fallback __cxa_thread_atexit() implementation when __cxa_thread_atexit_impl() isn't available. The same bug reproduces with --stl=libc++, but since the NDK's libc++ doesn't have __cxa_thread_atexit() (even when targeting API 23 for some reason), I have to provide my own to get it to build.

There was actually a patch recently made to libc++abi adding support for this on platforms that don't have native support, so once that lands and we pick it up we could actually support that on older platforms.

If you're talking about http://reviews.llvm.org/D21803, then that's my patch :). Hope it lands soon and you guys pick it up. For now I'm just rolling it into my own codebase.

[DanAlbert]

We've gotten our libc++ updates in shape now, so we should be able to pick it up whenever it lands. Will definitely be a great thing to have. Thanks for contributing!

[tavianator]

I thought I might be able to work around that LLVM bug with -femulated-tls but it seems to be ignored when -flto is passed.

[tavianator]

https://reviews.llvm.org/D21803 is merged now FYI

[DanAlbert]

I may not have time to do this for r14 given that unified headers still aren't quite ready, but I'll aim for that.

[tavianator]

@DanAlbert Okay cool, should we split that into a separate issue?

CANNOT LINK EXECUTABLE: cannot locate symbol "" referenced by "/data/local/tmp/a.out"...
page record for 0xb6e7003c was not found (block_size=16)

still happens regardless.

[DanAlbert]

Ah, yes. Agreed. Opened #216.

[huyuguang]

The bug still exist in ndk r15b

[DanAlbert]

Actually, I just forgot to close the bug. The test case in #156 (comment) works now.