Panicking when used as global allocator in WASM #24
Comments
|
From debug mode, I get |
khoover
changed the title
|
This is also observable if you replace the static-based #[cfg(target_family = "wasm")]
#[global_allocator]
static ALLOCATOR: talc::Talck<talc::locking::AssumeUnlockable, talc::WasmHandler> = {
// static mut MEMORY: [std::mem::MaybeUninit<u8>; 128 * 1024 * 1024] =
// [std::mem::MaybeUninit::uninit(); 128 * 1024 * 1024];
// let span = talc::Span::from_base_size(unsafe { MEMORY.as_ptr() as *mut _ }, 128 * 1024 * 1024);
talc::Talc::new(unsafe { talc::WasmHandler::new() }).lock()
}; |
|
Ouch. I'll see what I can do. Hopefully I can get to bottom of this by tomorrow. Thanks for the details. |
|
I'm trying to run the WASM test, but How do I get this up and running? |
|
Ah, forgot that the pkg directory was gitignore'd, I've moved the runner into the js-benches directory now. |
|
Thanks. I haven't quite figured out the why of what's going wrong, but I've figured out the what, I think. The panic is caused in Specifically, hooking up a logging callback for the allocator yielded an odd subsequence:
Of course, that last operation violated the safety contract on The relevant code: fn bench_jasper_decode(bytes: &[u8]) -> f64 {
// changing this fixes the issue, see below
let encoded = base32768::alternative::encode(bytes)
.chunks(64)
.map(JsString::from_char_code)
.reduce(|a, b| a.concat(&b))
.unwrap();
// the problem only shows up if this line is also present, otherwise nothing wrong occurs
let local_str: String = encoded.clone().into();
// you can comment out everything else, it doesn't affect anything
...This change fixes the problem entirely, no more bad deallocation: - let encoded = base32768::alternative::encode(bytes)
- .chunks(64)
- .map(JsString::from_char_code)
- .reduce(|a, b| a.concat(&b))
- .unwrap();
+ let encoded = arr_to_str(base32768::alternative::encode(bytes));I don't see what's wrong with the original code. For starters, it's completely safe as far as I can tell, so it causing UB is pretty sinister. Could this be a bug in Rust? No idea. I'll poke around some more tomorrow. |
|
Addendum: it's worth noting that while I can't easily get a list of allocator operations when using the default WASM global allocator, dlmalloc, it's fairly safe to say it's the same. Why doesn't dlmalloc exhibit any issues then? Because its a direct port of a C allocator, and C allocators don't take a size, just Not checking the coherence of the allocation size isn't strictly wrong but I think they should be checking this in debug builds because it invites safety violations. A similar issue cropped up here #17 where bad data passed to |
|
So, one thing here is that the UTF-16 sequence emitted by that encode function can have unpaired surrogates, if that impacts anything? It's strange that the version using arr_to_str doesn't have this problem, though, since it's almost a 1:1 translation of the Rust impl into JS (replacing |
|
I'm not sure. Perhaps of note, A is 1600000, B is 4800000, C is 4694072. (My earlier statement that C > B was a mistake.) Anyhow, a wasm-bindgen bug seems plausible, submitted there sounds like a solid idea. I'll close this issue as Talc is in the clear, but feel free to comment further. |
That pattern would mean the bad allocation is indeed function passStringToWasm0(arg, malloc, realloc) {
if (typeof(arg) !== 'string') throw new Error('expected a string argument');
if (realloc === undefined) {
const buf = cachedTextEncoder.encode(arg);
const ptr = malloc(buf.length, 1) >>> 0;
getUint8Memory0().subarray(ptr, ptr + buf.length).set(buf);
WASM_VECTOR_LEN = buf.length;
return ptr;
}
let len = arg.length;
let ptr = malloc(len, 1) >>> 0; // Where the A alloc happens
const mem = getUint8Memory0();
let offset = 0;
for (; offset < len; offset++) {
const code = arg.charCodeAt(offset);
if (code > 0x7F) break;
mem[ptr + offset] = code;
}
if (offset !== len) {
if (offset !== 0) {
arg = arg.slice(offset);
}
ptr = realloc(ptr, len, len = offset + arg.length * 3, 1) >>> 0; // Where the realloc to B happens
const view = getUint8Memory0().subarray(ptr + offset, ptr + len);
const ret = encodeString(arg, view);
if (ret.read !== arg.length) throw new Error('failed to pass whole string');
offset += ret.written;
}
WASM_VECTOR_LEN = offset;
return ptr;
}Looking at it, what I think the bug is is that bindgen doesn't give the Rust side the right capacity? Unless it tries to infer it from the length of the JS string plus the number of bytes decoded? |
Hi, I'm able to produce a memory access OOB error when using Talck as the global allocator, using nearly unsafe-free code (the bits I have are copy + paste from std from things like
ReadBuf). Here's the branch I can consistently UB from, using cargo on 1.74, exact console output is:When using the global allocator, the program runs to completion.
The text was updated successfully, but these errors were encountered: