rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.71k stars 12.75k forks source link

assertion failed: key as usize != KEY_SENTVAL in DragonFly BSD #112180

Open tuxillo opened 1 year ago

tuxillo commented 1 year ago

When running the rust-cbindgen binary I get the following error:

root@dflyvm# cbindgen --version
fatal runtime error: assertion failed: key as usize != KEY_SENTVAL
Abort (core dumped)

After asking around in some IRC channels and in #contribute in discord, the smoking gun seemed this PR: https://github.com/rust-lang/rust/pull/105359

Some more digging revealed that DragonFly BSD's pthread_key_create() behaves differently depending on whether the resulting binary is linked against libpthread or not, just like in FreeBSD, for example. In DragonFly BSD, for single-threaded programs the key is untouched (undefined behaviour) and the pthread_key_create() will be successful whereas FreeBSD returns an error.

The below example illustrates it:

Test program:

#include <stdio.h>
#include <pthread.h>

pthread_key_t key;

int
main(void) {

    for (int i = 0; i < 8; i++) {
        int rc;
        rc = pthread_key_create(&key, NULL);
        printf("k=%d rc=%d\n", key, rc);
    }
}

FreeBSD

no pthread linkage:

root@fbsdvm:~/temp # cc -o p p.c
root@fbsdvm:~/temp # ./p
k=0 rc=78
k=0 rc=78
k=0 rc=78
k=0 rc=78
k=0 rc=78
k=0 rc=78
k=0 rc=78
k=0 rc=78

pthread linkage:

root@fbsdvm:~/temp # ./p
k=1 rc=0
k=2 rc=0
k=3 rc=0
k=4 rc=0
k=5 rc=0
k=6 rc=0
k=7 rc=0
k=8 rc=0

DragonFly BSD

no pthread linkage:

root@dflyvm:~/temp #  ./p
k=0 rc=0
k=0 rc=0
k=0 rc=0
k=0 rc=0
k=0 rc=0
k=0 rc=0
k=0 rc=0
k=0 rc=0

pthread linkage:

root@dflyvm:~/temp #  ./p
k=2 rc=0
k=3 rc=0
k=4 rc=0
k=5 rc=0
k=6 rc=0
k=7 rc=0
k=8 rc=0
k=9 rc=0

Since the sentinel here is set to 0 and as DragonFly BSD always returns undefined (if initialized or in bss it will be 0, for the most part) for programs not linked against libpthread, then the assertion in lazy_init() is hit: https://github.com/rust-lang/rust/blob/1.68.2/library/std/src/sys_common/thread_local_key.rs#L122

Additionally, I have found that FreeBSD links their rust-cbindgen binary against pthread whereas we don't do that for DragonFly BSD, which I think would have covered the issue for them.

As a note, we're discussing in the DragonFly BSD side whether we should start our keys at index 1 or not, but my personal opinion is that in this context is that it would just mask the issue already pointed out in the lazy_init() comment.

Meta

rustc --version --verbose:

rustc 1.68.2 (9eb3afe9e 2023-03-27) (built from a source tarball)
binary: rustc
commit-hash: 9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0
commit-date: 2023-03-27
host: x86_64-unknown-dragonfly
release: 1.68.2
LLVM version: 15.0.6
Backtrace

``` N/A ```

Jules-Bertholet commented 1 year ago

@rustbot label O-dragonfly

joboet commented 1 year ago

105359 cannot have caused the regression, it only outlined the sentinel value to a constant. The check against zero was introduced in a9c1152 (before 1.0).

I do think that panicking is the right thing to do here, because returning the same id twice means that the key generation definitely failed. Adding a message to the assertion should make this less obfuscated.

Sidenote: IMHO it would be great if DragonFly reported this by returning e.g. ENOSYS, that would allow for nicer error handling.

tuxillo commented 1 year ago

I'm going to check why we return what we do. Also I'll check why FreeBSD does it differently and I'll report back.

tuxillo commented 1 year ago

Still pending on my side, please don't close.

joshtriplett commented 6 months ago

ping @tuxillo? Any update?