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

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

Open
tuxillo opened this issue Jun 1, 2023 · 5 comments
Open

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

tuxillo opened this issue Jun 1, 2023 · 5 comments
Labels
C-bug Category: This is a bug. O-dragonfly Operating system: DragonFly BSD regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@tuxillo
Copy link
Contributor

tuxillo commented Jun 1, 2023

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: #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

@tuxillo tuxillo added the C-bug Category: This is a bug. label Jun 1, 2023
@Jules-Bertholet
Copy link
Contributor

@rustbot label O-dragonfly

@rustbot rustbot added the O-dragonfly Operating system: DragonFly BSD label Jun 1, 2023
@jyn514 jyn514 added T-libs Relevant to the library team, which will review and decide on the PR/issue. regression-untriaged Untriaged performance or correctness regression. labels Jun 1, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 1, 2023
@jyn514 jyn514 added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-untriaged Untriaged performance or correctness regression. labels Jun 1, 2023
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 6, 2023
@joboet
Copy link
Member

joboet commented Jun 7, 2023

#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
Copy link
Contributor Author

tuxillo commented Jun 19, 2023

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
Copy link
Contributor Author

tuxillo commented Oct 26, 2023

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

@joshtriplett
Copy link
Member

ping @tuxillo? Any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-dragonfly Operating system: DragonFly BSD regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants