Skip to content

Commit

Permalink
Merge pull request #1692 from alexcrichton/fix-anyref-bugs
Browse files Browse the repository at this point in the history
Fix a number of `anyref` related bugs in our passes
  • Loading branch information
alexcrichton authored Aug 2, 2019
2 parents 36db788 + adde6c2 commit 117fce1
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 64 deletions.
35 changes: 2 additions & 33 deletions crates/anyref-xform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,6 @@ impl Transform<'_> {
// functions and make sure everything is still hooked up right.
self.rewrite_calls(module);

// Inject initialization routine to set up default slots in the table
// (things like null/undefined/true/false)
self.inject_initialization(module);

Ok(())
}

Expand Down Expand Up @@ -514,14 +510,14 @@ impl Transform<'_> {
// Store an anyref at an offset from our function's stack
// pointer frame.
let get_fp = builder.local_get(fp);
next_stack_offset += 1;
let (index, idx_local) = if next_stack_offset == 1 {
let (index, idx_local) = if next_stack_offset == 0 {
(get_fp, fp)
} else {
let rhs = builder.i32_const(next_stack_offset);
let add = builder.binop(BinaryOp::I32Add, get_fp, rhs);
(builder.local_tee(scratch_i32, add), scratch_i32)
};
next_stack_offset += 1;
let store = builder.table_set(self.table, index, local);
let get = builder.local_get(idx_local);
builder.with_side_effects(vec![store], get, Vec::new())
Expand Down Expand Up @@ -669,31 +665,4 @@ impl Transform<'_> {
}
}
}

// Ensure that the `start` function for this module calls the
// `__wbindgen_init_anyref_table` function. This'll ensure that all
// instances of this module have the initial slots of the anyref table
// initialized correctly.
fn inject_initialization(&mut self, module: &mut Module) {
let ty = module.types.add(&[], &[]);
let (import, _) = module.add_import_func(
"__wbindgen_placeholder__",
"__wbindgen_init_anyref_table",
ty,
);

let prev_start = match module.start {
Some(f) => f,
None => {
module.start = Some(import);
return;
}
};

let mut builder = walrus::FunctionBuilder::new();
let call_init = builder.call(import, Box::new([]));
let call_prev = builder.call(prev_start, Box::new([]));
let new_start = builder.finish(ty, Vec::new(), vec![call_init, call_prev], module);
module.start = Some(new_start);
}
}
2 changes: 1 addition & 1 deletion crates/cli-support/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ intrinsics! {
#[symbol = "__wbindgen_anyref_heap_live_count"]
#[signature = fn() -> I32]
AnyrefHeapLiveCount,
#[symbol = "__wbindgen_init_nyref_table"]
#[symbol = "__wbindgen_init_anyref_table"]
#[signature = fn() -> Unit]
InitAnyrefTable,
}
Expand Down
31 changes: 19 additions & 12 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,7 @@ impl<'a> Context<'a> {
// up we always remove the `start` function if one is present. The JS
// bindings glue then manually calls the start function (if it was
// previously present).
let mut needs_manual_start = false;
if self.config.emit_start {
needs_manual_start = self.unstart_start_function();
}
let needs_manual_start = self.unstart_start_function();

// After all we've done, especially
// `unexport_unused_internal_exports()`, we probably have a bunch of
Expand Down Expand Up @@ -517,7 +514,10 @@ impl<'a> Context<'a> {
for (i, extra) in extra_modules.iter().enumerate() {
let imports = match &mut imports {
Some(list) => list,
None => bail!("cannot import from modules (`{}`) with `--no-modules`", extra),
None => bail!(
"cannot import from modules (`{}`) with `--no-modules`",
extra
),
};
imports.push_str(&format!("import * as __wbg_star{} from '{}';\n", i, extra));
imports_init.push_str(&format!("imports['{}'] = __wbg_star{};\n", extra, i));
Expand Down Expand Up @@ -2641,16 +2641,23 @@ impl<'a> Context<'a> {

Intrinsic::InitAnyrefTable => {
self.expose_anyref_table();
String::from(

// Grow the table to insert our initial values, and then also
// set the 0th slot to `undefined` since that's what we've
// historically used for our ABI which is that the index of 0
// returns `undefined` for types like `None` going out.
let mut base = format!(
"
const table = wasm.__wbg_anyref_table;
const offset = table.grow(4);
table.set(offset + 0, undefined);
table.set(offset + 1, null);
table.set(offset + 2, true);
table.set(offset + 3, false);
const offset = table.grow({});
table.set(0, undefined);
",
)
INITIAL_HEAP_VALUES.len(),
);
for (i, value) in INITIAL_HEAP_VALUES.iter().enumerate() {
base.push_str(&format!("table.set(offset + {}, {});\n", i, value));
}
base
}
};
Ok(expr)
Expand Down
8 changes: 1 addition & 7 deletions crates/cli-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl Bindgen {
// the webidl bindings proposal) as well as an auxiliary section for all
// sorts of miscellaneous information and features #[wasm_bindgen]
// supports that aren't covered by WebIDL bindings.
webidl::process(&mut module)?;
webidl::process(&mut module, self.anyref, self.emit_start)?;

// Now that we've got type information from the webidl processing pass,
// touch up the output of rustc to insert anyref shims where necessary.
Expand All @@ -324,12 +324,6 @@ impl Bindgen {
anyref::process(&mut module)?;
}

// If we're in a testing mode then remove the start function since we
// shouldn't execute it.
if !self.emit_start {
module.start = None;
}

let aux = module
.customs
.delete_typed::<webidl::WasmBindgenAux>()
Expand Down
66 changes: 56 additions & 10 deletions crates/cli-support/src/webidl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,10 +473,14 @@ struct Context<'a> {
vendor_prefixes: HashMap<String, Vec<String>>,
unique_crate_identifier: &'a str,
descriptors: HashMap<String, Descriptor>,
anyref_enabled: bool,
support_start: bool,
}

pub fn process(
module: &mut Module,
anyref_enabled: bool,
support_start: bool,
) -> Result<(NonstandardWebidlSectionId, WasmBindgenAuxId), Error> {
let mut storage = Vec::new();
let programs = extract_programs(module, &mut storage)?;
Expand All @@ -491,6 +495,8 @@ pub fn process(
unique_crate_identifier: "",
module,
start_found: false,
anyref_enabled,
support_start,
};
cx.init()?;

Expand Down Expand Up @@ -532,18 +538,11 @@ impl<'a> Context<'a> {
}
}
for (id, intrinsic) in intrinsics {
bindings::register_import(
self.module,
&mut self.bindings,
id,
intrinsic.binding(),
ast::WebidlFunctionKind::Static,
)?;
self.aux
.import_map
.insert(id, AuxImport::Intrinsic(intrinsic));
self.bind_intrinsic(id, intrinsic)?;
}

self.inject_anyref_initialization()?;

if let Some(custom) = self
.module
.customs
Expand Down Expand Up @@ -612,6 +611,48 @@ impl<'a> Context<'a> {
Ok(())
}

// Ensure that the `start` function for this module calls the
// `__wbindgen_init_anyref_table` function. This'll ensure that all
// instances of this module have the initial slots of the anyref table
// initialized correctly.
fn inject_anyref_initialization(&mut self) -> Result<(), Error> {
if !self.anyref_enabled {
return Ok(());
}
let ty = self.module.types.add(&[], &[]);
let (import, import_id) = self.module.add_import_func(
PLACEHOLDER_MODULE,
"__wbindgen_init_anyref_table",
ty,
);

self.module.start = Some(match self.module.start {
Some(prev_start) => {
let mut builder = walrus::FunctionBuilder::new();
let call_init = builder.call(import, Box::new([]));
let call_prev = builder.call(prev_start, Box::new([]));
builder.finish(ty, Vec::new(), vec![call_init, call_prev], self.module)
}
None => import,
});
self.bind_intrinsic(import_id, Intrinsic::InitAnyrefTable)?;
Ok(())
}

fn bind_intrinsic(&mut self, id: ImportId, intrinsic: Intrinsic) -> Result<(), Error> {
bindings::register_import(
self.module,
&mut self.bindings,
id,
intrinsic.binding(),
ast::WebidlFunctionKind::Static,
)?;
self.aux
.import_map
.insert(id, AuxImport::Intrinsic(intrinsic));
Ok(())
}

fn program(&mut self, program: decode::Program<'a>) -> Result<(), Error> {
self.unique_crate_identifier = program.unique_crate_identifier;
let decode::Program {
Expand Down Expand Up @@ -752,6 +793,11 @@ impl<'a> Context<'a> {
}
self.start_found = true;

// Skip this when we're generating tests
if !self.support_start {
return Ok(());
}

let prev_start = match self.module.start {
Some(f) => f,
None => {
Expand Down
2 changes: 1 addition & 1 deletion src/anyref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl Slab {
internal_error("table grow failure")
}
if self.base == 0 {
self.base = r as usize + (super::JSIDX_RESERVED as usize);
self.base = r as usize;
} else if self.base + self.data.len() != r as usize {
internal_error("someone else allocated table entires?")
}
Expand Down

0 comments on commit 117fce1

Please sign in to comment.