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.

Sign up for GitHub

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
Merged

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

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7c307c0
Support threads in the new crt1-command.c ctor check.
sunfishcode Oct 14, 2022
e3a04a8
Fix the type of `started`.
sunfishcode Oct 14, 2022
15c74f9
Factor out common code.
sunfishcode Oct 14, 2022
cec75b4
Use <stdatomic.h> instead of __sync_ intrinsics.
sunfishcode Oct 14, 2022
69200f8
Add a comment about `volatile`.
sunfishcode Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions libc-bottom-half/crt/crt1-command.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
#ifdef _REENTRANT
#include <stdatomic.h>
#endif
#include <wasi/api.h>
extern void __wasm_call_ctors(void);
extern int __main_void(void);
extern void __wasm_call_dtors(void);

// Commands should only be called once per instance. This simple check ensures
// that the `_start` function isn't started more than once.
static volatile int started = 0;

__attribute__((export_name("_start")))
void _start(void) {
// Don't allow the program to be called multiple times.
// Commands should only be called once per instance. This simple check
// ensures that the `_start` function isn't started more than once.
//
// We use `volatile` here to prevent the store to `started` from being
// sunk past any subsequent code, and to prevent any compiler from
// optimizing based on the knowledge that `_start` is the program
// entrypoint.
#ifdef _REENTRANT
static volatile _Atomic int started = 0;
int expected = 0;
if (!atomic_compare_exchange_strong(&started, &expected, 1)) {
__builtin_trap();
}
#else
static volatile int started = 0;
sunfishcode marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@sbc100 sbc100 Oct 14, 2022 •
edited
Loading

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.

if (started != 0) {
__builtin_trap();
}
started = 1;
#endif

// The linker synthesizes this to call constructors.
__wasm_call_ctors();
Expand Down