Skip to content

Instantly share code, notes, and snippets.

@thomcc
Created June 5, 2023 16:10
Show Gist options
  • Save thomcc/8c4a8003cf1a282949a539e899cb45aa to your computer and use it in GitHub Desktop.
Save thomcc/8c4a8003cf1a282949a539e899cb45aa to your computer and use it in GitHub Desktop.

__dso_handle misuse

This was going to be part of a comment in rust-lang/rust#110897, but now is not.

The full writeup is in rust-lang/rust#88737, although you have to read several comments to get to full understanding, and even then I'm not sure I've written down everything, so here it is*

Basically we use __dso_handle totally wrong right now in this code. This causes bad things to happen (stale fn ptrs get called, usually) when people dlclose Rust modules that have registered TLS dtors. This is not something we can ever fully support though, but the bad behavior is spooky enough that IMO we should just use these APIs correctly.

For background: The __dso_handle is a magic symbol that has a different value in every .so (it's a hidden symbol provided by the loader). It's used in this case to let the runtime ("the runtime" = libc + loader + ...) know which shared library the thread local dtor applies to, which allows the runtime to avoid some cases where it would call a function pointer coming from an unloaded module. In practice, it will usually just avoid unloading the module if it has outstanding TLS dtors.

There are two ways we're doing this wrong:

  1. We misuse extern_weak for __dso_handle. extern_weak for statics always adds a layer of indirection, so our &__dso_handle as *mut _ as ... is a double-pointer). That said, there's no reason to use extern_weak on __dso_handle on these targets, it should always be present (Note: I say this with like, 95% confidence...).

    This matters mainly for the case where you have a cdylib with a statically linked libstd, and want to unload the whole bundle. Now that I type this out, it seems like it's the more important one to fix compared to the second.

  2. Even if that were fixed, we'd still be passing the __dso_handle for libstd rather than for the crate where the thread_local is defined.

    This matters if your program uses both dynamic linking (Not common in Rust, altho #[export] might change that... 👀) and dlclose together, especially if std is dynamically linked but isn't being unloaded (to be clear: std is not being unloaded in this scenario)

The first is easy to fix (easiest is to make __dso_handle not an extern_weak, and should be possible if we want to keep the (almost certainly unnecessary) weak import (I'm not sure concretely but it would either be passing __dso_handle directly, or doing something like &**__dso_handle maybe? IDK, extern_weak on statics is really jank, honestly).

The 2nd is not hard, just really annoying, since it means thread_local! on these targets needs to expand to include an extern "C" { static __dso_handle } and thread its pointer though.

All that said, failing to fix this just brings us back to the status quo1 that we have on most targets, since we often either don't have a unload-aware function for registering TLS dtors, or we do have one, but can't use it for one reason or another (for example, reentrancy concerns).

TLDR

  • __dso_handle has an extra level of indirection caused by how we implement extern_weak for statics.

  • Even if that were fixed, we always provide std's __dso_handle, rather than the one for the user's crate. This would be wrong in code that dynamically links things (especially if std itself is dynamically linked).

  • This matters only in code that dlcloses Rust modules. but if we're considering redesigns it would be good to fix, as usually the consequence of this going bad is that a stale function pointer gets called, which is highly spooky. That said, most platforms are in this boat, so it's basically a QOI issue -- people should just stop calling dlclose.

Footnotes

  1. Of "it's probably not supported for you to dlclose/FreeLibrary on rust code unless it's #[no_std]" -- Honestly, given the existence of 'static, IMO this is broadly true for all Rust code dlclose cannot be safe.

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