Skip to content

Commit 3d6f736

Browse files
alexcrichtonalexcrichton
committed
Fix symbol conflicts defined in two crates
Not a lot of attention has been paid to dealing with conflicts of symbols between crates and different `#[wasm_bindgen]` blocks. This commit starts to fix this issue by unblocking #486 which first ran into this. Currently there's a bug where if two independent crates bind the same JS API they'll generate the same symbol which causes conflicts for things like LTO or linking in general. This commit starts to add a "salt" to all symbols generated by `wasm-bindgen` (these are all transparent to the user) to ensure that each crate's invocations are kept apart from one another and using the correct bindings.
1 parent 9218c40 commit 3d6f736

File tree

5 files changed

+168
-33
lines changed

5 files changed

+168
-33
lines changed

‎crates/backend/src/codegen.rs

+3-28
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
use std::borrow::Cow;
21
use std::collections::HashSet;
3-
use std::env;
42
use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
53

64
use ast;
@@ -9,24 +7,7 @@ use quote::ToTokens;
97
use serde_json;
108
use shared;
119
use syn;
12-
13-
fn to_ident_name(s: &str) -> Cow<str> {
14-
if s.chars().all(|c| match c {
15-
'a'...'z' | 'A'...'Z' | '0'...'9' | '_' => true,
16-
_ => false,
17-
}) {
18-
return Cow::from(s);
19-
}
20-
21-
Cow::from(
22-
s.chars()
23-
.map(|c| match c {
24-
'a'...'z' | 'A'...'Z' | '0'...'9' | '_' => c,
25-
_ => '_',
26-
})
27-
.collect::<String>(),
28-
)
29-
}
10+
use util::ShortHash;
3011

3112
impl ToTokens for ast::Program {
3213
// Generate wrappers for all the items that we've found
@@ -72,15 +53,9 @@ impl ToTokens for ast::Program {
7253

7354
static CNT: AtomicUsize = ATOMIC_USIZE_INIT;
7455

75-
let crate_name = env::var("CARGO_PKG_NAME").expect("should have CARGO_PKG_NAME env var");
76-
let crate_vers =
77-
env::var("CARGO_PKG_VERSION").expect("should have CARGO_PKG_VERSION env var");
78-
7956
let generated_static_name = format!(
80-
"__WASM_BINDGEN_GENERATED_{}_{}_{}",
81-
to_ident_name(&crate_name),
82-
to_ident_name(&crate_vers),
83-
CNT.fetch_add(1, Ordering::SeqCst)
57+
"__WASM_BINDGEN_GENERATED_{}",
58+
ShortHash(CNT.fetch_add(1, Ordering::SeqCst)),
8459
);
8560
let generated_static_name = Ident::new(&generated_static_name, Span::call_site());
8661

‎crates/backend/src/util.rs

+41
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1+
use std::collections::hash_map::DefaultHasher;
2+
use std::env;
3+
use std::fmt;
4+
use std::hash::{Hash, Hasher};
15
use std::iter::FromIterator;
6+
use std::sync::atomic::Ordering::SeqCst;
7+
use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT};
8+
use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT};
29

310
use ast;
411
use proc_macro2::{self, Ident};
@@ -88,3 +95,37 @@ pub fn wrap_import_function(function: ast::ImportFunction) -> ast::Import {
8895
kind: ast::ImportKind::Function(function),
8996
}
9097
}
98+
99+
/// Small utility used when generating symbol names.
100+
///
101+
/// Hashes the public field here along with a few cargo-set env vars to
102+
/// distinguish between runs of the procedural macro.
103+
pub struct ShortHash<T>(pub T);
104+
105+
impl<T: Hash> fmt::Display for ShortHash<T> {
106+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
107+
static HASHED: AtomicBool = ATOMIC_BOOL_INIT;
108+
static HASH: AtomicUsize = ATOMIC_USIZE_INIT;
109+
110+
// Try to amortize the cost of loading env vars a lot as we're gonna be
111+
// hashing for a lot of symbols.
112+
if !HASHED.load(SeqCst) {
113+
let mut h = DefaultHasher::new();
114+
env::var("CARGO_PKG_NAME")
115+
.expect("should have CARGO_PKG_NAME env var")
116+
.hash(&mut h);
117+
env::var("CARGO_PKG_VERSION")
118+
.expect("should have CARGO_PKG_VERSION env var")
119+
.hash(&mut h);
120+
// This may chop off 32 bits on 32-bit platforms, but that's ok, we
121+
// just want something to mix in below anyway.
122+
HASH.store(h.finish() as usize, SeqCst);
123+
HASHED.store(true, SeqCst);
124+
}
125+
126+
let mut h = DefaultHasher::new();
127+
HASH.load(SeqCst).hash(&mut h);
128+
self.0.hash(&mut h);
129+
write!(f, "{:016x}", h.finish())
130+
}
131+
}

‎crates/macro/src/parser.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
use backend::{ast, util::ident_ty};
1+
use backend::ast;
2+
use backend::util::{ident_ty, ShortHash};
23
use proc_macro2::{Ident, Span, TokenStream, TokenTree};
34
use quote::ToTokens;
45
use shared;
@@ -427,10 +428,11 @@ impl ConvertToAst<BindgenAttrs> for syn::ForeignItemFn {
427428

428429
let shim = {
429430
let ns = match kind {
430-
ast::ImportFunctionKind::Normal => "n",
431-
ast::ImportFunctionKind::Method { ref class, .. } => class,
431+
ast::ImportFunctionKind::Normal => (0, "n"),
432+
ast::ImportFunctionKind::Method { ref class, .. } => (1, &class[..]),
432433
};
433-
format!("__wbg_f_{}_{}_{}", js_name, self.ident, ns)
434+
let data = (ns, &self.ident);
435+
format!("__wbg_{}_{}", js_name, ShortHash(data))
434436
};
435437
ast::ImportKind::Function(ast::ImportFunction {
436438
function: wasm,

‎examples/julia_set/build.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
set -ex
66

7-
cargo +nightly build --target wasm32-unknown-unknown --release
7+
cargo +nightly build --target wasm32-unknown-unknown
88
cargo +nightly run --manifest-path ../../crates/cli/Cargo.toml \
99
--bin wasm-bindgen -- \
1010
../../target/wasm32-unknown-unknown/release/julia_set.wasm --out-dir .

‎tests/all/dependencies.rs

+117
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,120 @@ fn dependencies_work() {
9090
)
9191
.test();
9292
}
93+
94+
#[test]
95+
fn same_api_two_crates() {
96+
project()
97+
.file(
98+
"src/lib.rs",
99+
r#"
100+
#![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
101+
extern crate wasm_bindgen;
102+
extern crate a;
103+
extern crate b;
104+
105+
use wasm_bindgen::prelude::*;
106+
107+
#[wasm_bindgen(module = "./foo")]
108+
extern {
109+
fn assert_next_undefined();
110+
fn assert_next_ten();
111+
}
112+
113+
#[wasm_bindgen]
114+
pub fn test() {
115+
assert_next_undefined();
116+
a::test();
117+
assert_next_ten();
118+
b::test();
119+
}
120+
"#,
121+
)
122+
.file(
123+
"foo.js",
124+
r#"
125+
import { strictEqual } from "assert";
126+
127+
let next = null;
128+
129+
export function assert_next_undefined() {
130+
next = undefined;
131+
}
132+
133+
export function assert_next_ten() {
134+
next = 10;
135+
}
136+
137+
export function foo(a) {
138+
console.log(a, next);
139+
strictEqual(a, next);
140+
next = null;
141+
}
142+
"#,
143+
)
144+
.add_local_dependency("a", "a")
145+
.file(
146+
"a/Cargo.toml",
147+
&format!(r#"
148+
[package]
149+
name = 'a'
150+
version = '0.0.0'
151+
152+
[dependencies]
153+
wasm-bindgen = {{ path = '{}' }}
154+
"#,
155+
env!("CARGO_MANIFEST_DIR")
156+
),
157+
)
158+
.file(
159+
"a/src/lib.rs",
160+
"
161+
#![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
162+
extern crate wasm_bindgen;
163+
164+
use wasm_bindgen::prelude::*;
165+
166+
#[wasm_bindgen(module = \"./foo\")]
167+
extern {
168+
fn foo();
169+
}
170+
171+
pub fn test() {
172+
foo();
173+
}
174+
",
175+
)
176+
.add_local_dependency("b", "b")
177+
.file(
178+
"b/Cargo.toml",
179+
&format!(r#"
180+
[package]
181+
name = 'b'
182+
version = '0.0.0'
183+
184+
[dependencies]
185+
wasm-bindgen = {{ path = '{}' }}
186+
"#,
187+
env!("CARGO_MANIFEST_DIR")
188+
),
189+
)
190+
.file(
191+
"b/src/lib.rs",
192+
"
193+
#![feature(use_extern_macros, wasm_custom_section, wasm_import_module)]
194+
extern crate wasm_bindgen;
195+
196+
use wasm_bindgen::prelude::*;
197+
198+
#[wasm_bindgen(module = \"./foo\")]
199+
extern {
200+
fn foo(x: u32);
201+
}
202+
203+
pub fn test() {
204+
foo(10);
205+
}
206+
",
207+
)
208+
.test();
209+
}

0 commit comments

Comments
 (0)