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

Support threads in the new crt1-command.c ctor check. #339

Merged
merged 5 commits into from
Nov 8, 2022

Conversation

sunfishcode
Copy link
Member

Use an atomic compare-and-swap for checking whether constructors have been run, when threads are enabled.

Use an atomic compare-and-swap for checking whether constructors have
been run, when threads are enabled.
Copy link
Collaborator

@abrown abrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me.

@abrown
Copy link
Collaborator

abrown commented Oct 14, 2022

cc: @haraldh, this might be helpful for you.

// ensures that the `_start` function isn't started more than once.
static volatile int started = 0;
#ifdef _REENTRANT
if (__sync_val_compare_and_swap(&started, 0, 1) != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using C11 atomics here? (https://www.ibm.com/docs/en/zos/2.1.0?topic=c11-atomic-compare-exchange-strong). Maybe we don't want to depend on C11?

__builtin_trap();
}
#else
static volatile int started = 0;
Copy link
Member

@sbc100 sbc100 Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for volatile here I think? (or above actually?).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now added a comment about volatile. It's admittedly a very theoretical concern. But also, this variable is read and written to exactly once, so there's nothing to optimize about it. So I'm inclined to keep the volatile.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Persisting an in-memory variable past the point of an _exit call isn't something that's even possible to think about in C, and yet that's what this code is doing, so volatile helps make sure that the compiler just does what we're telling it to do, because it won't understand what we're actually doing.

@abrown abrown merged commit b99173e into main Nov 8, 2022
@abrown abrown deleted the sunfishcode/command-called-once branch November 8, 2022 21:37
john-sharratt pushed a commit to john-sharratt/wasix-libc that referenced this pull request Mar 6, 2023
Use an atomic compare-and-swap for checking whether constructors have
been run, when threads are enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants