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

Fix a number of anyref related bugs in our passes #1692

Merged
merged 4 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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"]
Copy link
Member

Choose a reason for hiding this comment

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

lolsob...

#[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