-
Notifications
You must be signed in to change notification settings - Fork 16
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
failing compilation with musl #102
Comments
At'ing @mjp41 for visibility in case similar musl issues occur with other snmalloc bindings in other PLs |
@davidchisnall any thoughts? |
Last time I looked at the musl, it didn't correctly handle library unloading even with shared libraries. It also doesn't implement It is probably fine to just define something like this for static linking:
You might actually be able to get away with a weak symbol: it doesn't matter if it resolves to 0, because the C++ runtime won't do anything with it other than equality comparisons. It would be nice if more libc implementations had the explicit 'destroy malloc state for this thread' hook that FreeBSD libc provides. |
So I added #[no_mangle]
pub static __dso_handle: u8 = 0; to the mainline, that resolves the compilation issue but causes it to segfault:
|
It looks as if this is in the registration of the TLS destructor for |
I've spent a lot of effort making sure things get tail called ;-) |
I think this is inside the C++ runtime, not yours. |
It looks like this would be the first place |
Can we build the |
but I remember rust itself relies on \_\_cxa\_thread\_atexit to handle the thread\_local dtor (see std library) . Did it specialize the behavior for MUSL?
\-------- Original Message --------
On Jan 27, 2021, 9:55 PM, David Chisnall < ***@***.***> wrote:
`__dso_handle` is a magic symbol that's expected to be present with hidden visibility in every ELF shared library. It's passed to the C++ runtime function that handles destructors so that, when a library is unloaded all of the destructors can be run (and all of the thread-local destructors can be skipped if the library has been unloaded). It's usually provided by the CSU parts and might be missing with MUSL for statically linked binaries. I don't know why MUSL doesn't define it, because the C++ ABI requires that it exists, even for static linking.
Last time I looked at the musl, it [didn't correctly handle library unloading][didn_t correctly handle library unloading] even with shared libraries. It also doesn't implement `__cxa_thread_atexit_impl` or `__cxa_thread_atexit` at all, so the C++ runtime library has to fall back to using `pthread_setspecific` or similar for thread-local cleanup. I don't know what the implication of that is for snmalloc: It doesn't look as if musl needs to call `free` during thread teardown after it runs the thread-local destructors, so it's probably fine.
It is *probably* fine to just define something like this for static linking:
```
__attribute__((__visibility__((hidden)))
char __dso_handle;
```
You might actually be able to get away with a weak symbol: it doesn't matter if it resolves to 0, because the C++ runtime won't do anything with it other than equality comparisons.
It would be nice if more libc implementations had the explicit 'destroy malloc state for this thread' hook that FreeBSD libc provides.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, [view it on GitHub][], or [unsubscribe][].![AEZNMJPUJWLGLPOGG37WKVTS4ALOXA5CNFSM4WVGVTC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFXFVRGQ.gif][]
[didn_t correctly handle library unloading]: https://github.com/bminor/musl/blob/3ab2a4e02682df1382955071919d8aa3c3ec40d4/src/exit/atexit.c#L39
[view it on GitHub]: #102 (comment)
[unsubscribe]: https://github.com/notifications/unsubscribe-auth/AEZNMJOPQ6JGKC4ALJHJSUDS4ALOXANCNFSM4WVGVTCQ
[AEZNMJPUJWLGLPOGG37WKVTS4ALOXA5CNFSM4WVGVTC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFXFVRGQ.gif]: https://github.com/notifications/beacon/AEZNMJPUJWLGLPOGG37WKVTS4ALOXA5CNFSM4WVGVTC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFXFVRGQ.gif
|
Out of the box, sadly that isn't defined it seems, lldb doesn't find that breakpoint.
Yap I created a branch hat includes the bits needed for musl https://github.com/Licenser/snmalloc-rs/tree/musl
runs the tests with musl and has the same results as the example repo |
Is it possible to link this with a debug build of musl and whatever C++ runtime you need (or can you install packages for these with separate debug info, if they're dynamically linked)? The bug seems to be in the interaction between clang-generated code from standard C++ language constructs, calling standard Itanium C++ runtime functions, and their implementations in the C++ runtime and libc. |
Thou it may be irrelevant, but in my build script I required libstdc++ to be dynamic explicitly. I should have it fixed. |
It looks as if Rust does this by using |
So we use to have a version that used in the There is an awkward corner case that the destructor code for the thread has cleaned up the allocator, but other destructors will still call the allocator. This is handled by having a thread local that knows whether the destructor has run, and then moves to a very slow path for all allocations and deallocations. Happy to help if someone wants to do the work. The pthread version was removed in |
if we handle this by ourselves, I think that it will also be a great workaround for the MinGW bug addressed in the open PR of snmalloc. |
microsoft/snmalloc#217 So I think you would need a to something like void* pthread_make_key()
{
pthread_key_t key;
pthread_key_create(&key, ThreadAllocCommon::inner_release);
return nullptr;
} and then static bool register_cleanup()
{
#ifndef SNMALLOC_USING_PTHREAD
static thread_local OnDestruct<ThreadAllocCommon::inner_release> tidier;
#else
// Don't need pthread_key_t as only want to use singleton to register destructor
Singleton<void*, pthread_make_key>::get();
#endif
ThreadAllocCommon::register_cleanup();
return destructor_has_run;
} |
The other option, of course, is for someone to contribute a |
Would that fix MingW? But fixing musl would be good. |
Yes. Basically, the mingw problem is caused by unable to free the |
@mjp41 :( well, it was too early for me to have that belief... After several trials, I still cannot fix |
@Licenser is the MUSL problem still an issue for you? We could try this as a work-around until MUSL is fixed. |
Yes but not with a super high priority. It prevents us to make statically linked tremor binaries, which is something we'd love to have, OTOH musl was significantly slower for us in the past when builds were working so they'd just be an addition not 'the' release strategy. I think it's totally fine to wait for a proper fix and not put a workaround on your plate :) we're already super thankful that you all got to the bottom of this! |
Hi,
I'm trying to build an application using snmalloc in with a statically linked musl libc instead of the gnu libc, during that we've been running in a few problems.
I am not sure 100% where the issue orriginates, from my research it seems to be related to the usage of
C++
and global destructors.The error boils down to:
To make it easy I've created a minimal reproduction with nothing but snmalloc-rs and a mainline function:
https://github.com/Licenser/snmalloc-musl
The error can be triggered by simply running
make
, it will build a musl docker image and then try to compile the application in it.The text was updated successfully, but these errors were encountered: