Skip to content

Commit

Permalink
Fix logic for is_enabled in the threads transform (#1791)
Browse files Browse the repository at this point in the history
The threads transform is implicitly enabled nowadays when the memory
looks like it's shared, so ensure that's taken into account in the
`is_enabled` check.
  • Loading branch information
alexcrichton authored Sep 25, 2019
1 parent f4a7fe3 commit 6f52f2a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/cli-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ impl Bindgen {
// pointer, so temporarily export it so that our many GC's don't remove
// it before the xform runs.
let mut exported_shadow_stack_pointer = false;
if self.multi_value || self.threads.is_enabled() {
if self.multi_value || self.threads.is_enabled(&module) {
wasm_conventions::export_shadow_stack_pointer(&mut module)?;
exported_shadow_stack_pointer = true;
}
Expand Down
29 changes: 16 additions & 13 deletions crates/threads-xform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,21 @@ impl Config {
}

/// Is threaded Wasm enabled?
pub fn is_enabled(&self) -> bool {
self.enabled
pub fn is_enabled(&self, module: &Module) -> bool {
if self.enabled {
return true;
}

// Compatibility with older LLVM outputs. Newer LLVM outputs, when
// atomics are enabled, emit a shared memory. That's a good indicator
// that we have work to do. If shared memory isn't enabled, though then
// this isn't an atomic module so there's nothing to do. We still allow,
// though, an environment variable to force us to go down this path to
// remain compatibile with older LLVM outputs.
match wasm_conventions::get_memory(module) {
Ok(memory) => module.memories.get(memory).shared,
Err(_) => false,
}
}

/// Specify the maximum amount of memory the wasm module can ever have.
Expand Down Expand Up @@ -87,21 +100,11 @@ impl Config {
///
/// More and/or less may happen here over time, stay tuned!
pub fn run(&self, module: &mut Module) -> Result<(), Error> {
if !self.enabled {
if !self.is_enabled(module) {
return Ok(());
}

// Compatibility with older LLVM outputs. Newer LLVM outputs, when
// atomics are enabled, emit a shared memory. That's a good indicator
// that we have work to do. If shared memory isn't enabled, though then
// this isn't an atomic module so there's nothing to do. We still allow,
// though, an environment variable to force us to go down this path to
// remain compatibile with older LLVM outputs.
let memory = wasm_conventions::get_memory(module)?;
if !module.memories.get(memory).shared {
return Ok(());
}

let stack_pointer = wasm_conventions::get_shadow_stack_pointer(module)?;
let addr = allocate_static_data(module, memory, 4, 4)?;
let zero = InitExpr::Value(Value::I32(0));
Expand Down

0 comments on commit 6f52f2a

Please sign in to comment.