Skip to content

Conversation

@danielvallance
Copy link

@danielvallance danielvallance commented Nov 23, 2025

This commit adds suppport for emitting atomic instructions when wasm_smith::Config::threads_enabled and
wasm_smith::Config::shared_everything_threads_enabled are true.

A link to the atomic instructions in the threads proposal: https://webassembly.github.io/threads/core/syntax/instructions.html#syntax-instr-atomic-memory

Addresses #2384

@danielvallance danielvallance requested a review from a team as a code owner November 23, 2025 17:39
@danielvallance danielvallance requested review from dicej and removed request for a team November 23, 2025 17:39
@danielvallance danielvallance changed the title Generate atomic instructions defined in the threads proposal (https://github.com/bytecodealliance/wasm-tools/issues/2384) Generate atomic instructions defined in the threads proposal (#2384) Nov 23, 2025
@danielvallance danielvallance changed the title Generate atomic instructions defined in the threads proposal (#2384) Generate atomic instructions defined in the threads proposal Nov 23, 2025
@dicej
Copy link
Collaborator

dicej commented Nov 24, 2025

@abrown Would you have a few minutes to look at this? I'm not up-to-speed yet on shared-everything threads.

@tschneidereit
Copy link
Member

FYI, Andrew is on leave right now, so it might be a bit until he'll be able to look at this. (Not providing any timelines here because once he is back, he might have a lot to work through before getting to this for all I know.)

@alexcrichton alexcrichton requested review from alexcrichton and removed request for dicej November 24, 2025 15:39
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks for this! Could you also run the validate-valid-module fuzzer locally for a bit to double-check these are correct? That can be done with:

$ FUZZER=validate_valid_module cargo +nightly fuzz run -s none run

If nothing shows up for 10-15m I'd say it's good to go.

Comment on lines 7405 to 7409
#[inline]
fn atomic_instruction_valid(module: &Module, _builder: &mut CodeBuilder) -> bool {
module.config.threads_enabled && module.config.shared_everything_threads_enabled
}

Copy link
Member

Choose a reason for hiding this comment

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

For this it's ok to drop shared_everything_threads_enabled since these instructions only require the wasm threads proposal.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh ok I see, will do, thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Hi @alexcrichton , I have run this for 15 minutes and I did not see any issues apart from some warnings at the start which appear on the main branch also:

WARNING: Failed to find function "__sanitizer_acquire_crash_state". Reason dlsym(RTLD_DEFAULT, __sanitizer_acquire_crash_state): symbol not found.
WARNING: Failed to find function "__sanitizer_print_stack_trace". Reason dlsym(RTLD_DEFAULT, __sanitizer_print_stack_trace): symbol not found.
WARNING: Failed to find function "__sanitizer_set_death_callback". Reason dlsym(RTLD_DEFAULT, __sanitizer_set_death_callback): symbol not found.

Just to sanity check that I was running the tool correctly and that I could recognise an error, I also deliberately added a mistake to it and it fell over after a couple of seconds with a very clear and obvious error.

Would you like me to attach the output of the successful run?

This commit adds suppport for emitting atomic instructions when
wasm_smith::Config::threads_enabled and
wasm_smith::Config::shared_everything_threads_enabled are true.

A link to the atomic instructions in the threads proposal:
https://webassembly.github.io/threads/core/syntax/instructions.html#syntax-instr-atomic-memory
@danielvallance danielvallance force-pushed the threads_enabled_instructions branch from ae2f630 to 40bf650 Compare November 24, 2025 19:20
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.

4 participants