Skip to content

Commit f6d5a9d

Browse files
kripkenkripken
committed
Be more careful when loading i64 in wasm-js glue, we had a bug where the bits were trampled before we read them (WebAssembly#460)
1 parent da1b3b8 commit f6d5a9d

5 files changed

Lines changed: 13440 additions & 9608 deletions

File tree

‎bin/wasm.js‎

Lines changed: 13406 additions & 9596 deletions
Large diffs are not rendered by default.

‎build-js.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66

77
echo "building wasm.js"
88

9-
em++ -std=c++11 src/wasm-js.cpp src/pass.cpp src/passes/MergeBlocks.cpp src/passes/Print.cpp src/passes/OptimizeInstructions.cpp src/passes/RemoveUnusedBrs.cpp src/passes/RemoveUnusedNames.cpp src/passes/PostEmscripten.cpp src/passes/SimplifyLocals.cpp src/passes/ReorderLocals.cpp src/passes/Vacuum.cpp src/emscripten-optimizer/parser.cpp src/emscripten-optimizer/simple_ast.cpp src/emscripten-optimizer/optimizer-shared.cpp src/support/colors.cpp src/support/safe_integer.cpp src/support/bits.cpp src/support/threads.cpp src/asmjs/asm_v_wasm.cpp src/asmjs/shared-constants.cpp src/wasm.cpp -Isrc/ -o bin/wasm.js -s MODULARIZE=1 -s 'EXPORT_NAME="WasmJS"' --memory-init-file 0 -Oz -s ALLOW_MEMORY_GROWTH=1 -profiling -s DEMANGLE_SUPPORT=1 #-DWASM_JS_DEBUG -DWASM_INTERPRETER_DEBUG=2
9+
em++ -std=c++11 src/wasm-js.cpp src/passes/pass.cpp src/passes/MergeBlocks.cpp src/passes/Print.cpp src/passes/OptimizeInstructions.cpp src/passes/RemoveUnusedBrs.cpp src/passes/RemoveUnusedNames.cpp src/passes/PostEmscripten.cpp src/passes/SimplifyLocals.cpp src/passes/ReorderLocals.cpp src/passes/Vacuum.cpp src/emscripten-optimizer/parser.cpp src/emscripten-optimizer/simple_ast.cpp src/emscripten-optimizer/optimizer-shared.cpp src/support/colors.cpp src/support/safe_integer.cpp src/support/bits.cpp src/support/threads.cpp src/asmjs/asm_v_wasm.cpp src/asmjs/shared-constants.cpp src/wasm.cpp -Isrc/ -o bin/wasm.js -s MODULARIZE=1 -s 'EXPORT_NAME="WasmJS"' --memory-init-file 0 -Oz -s ALLOW_MEMORY_GROWTH=1 -profiling -s DEMANGLE_SUPPORT=1 #-DWASM_JS_DEBUG -DWASM_INTERPRETER_DEBUG=2
1010

1111
echo "building binaryen.js"
1212

1313
python ~/Dev/emscripten/tools/webidl_binder.py src/js/binaryen.idl glue
14-
em++ -std=c++11 src/binaryen-js.cpp src/pass.cpp src/passes/MergeBlocks.cpp src/passes/Print.cpp src/passes/RemoveUnusedBrs.cpp src/passes/RemoveUnusedNames.cpp src/passes/PostEmscripten.cpp src/passes/SimplifyLocals.cpp src/passes/ReorderLocals.cpp src/passes/Vacuum.cpp src/emscripten-optimizer/parser.cpp src/emscripten-optimizer/simple_ast.cpp src/emscripten-optimizer/optimizer-shared.cpp src/support/colors.cpp src/support/safe_integer.cpp src/support/bits.cpp src/support/threads.cpp src/asmjs/asm_v_wasm.cpp src/asmjs/shared-constants.cpp src/wasm.cpp -Isrc/ -o bin/binaryen.js -s MODULARIZE=1 -s 'EXPORT_NAME="Binaryen"' --memory-init-file 0 -Oz -s ALLOW_MEMORY_GROWTH=1 -profiling -s DEMANGLE_SUPPORT=1 -s INVOKE_RUN=0 --post-js glue.js
14+
em++ -std=c++11 src/binaryen-js.cpp src/passes/pass.cpp src/passes/MergeBlocks.cpp src/passes/Print.cpp src/passes/RemoveUnusedBrs.cpp src/passes/RemoveUnusedNames.cpp src/passes/PostEmscripten.cpp src/passes/SimplifyLocals.cpp src/passes/ReorderLocals.cpp src/passes/Vacuum.cpp src/emscripten-optimizer/parser.cpp src/emscripten-optimizer/simple_ast.cpp src/emscripten-optimizer/optimizer-shared.cpp src/support/colors.cpp src/support/safe_integer.cpp src/support/bits.cpp src/support/threads.cpp src/asmjs/asm_v_wasm.cpp src/asmjs/shared-constants.cpp src/wasm.cpp -Isrc/ -o bin/binaryen.js -s MODULARIZE=1 -s 'EXPORT_NAME="Binaryen"' --memory-init-file 0 -Oz -s ALLOW_MEMORY_GROWTH=1 -profiling -s DEMANGLE_SUPPORT=1 -s INVOKE_RUN=0 --post-js glue.js

‎src/wasm-js.cpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ extern "C" void EMSCRIPTEN_KEEPALIVE instantiate() {
165165
EM_ASM({
166166
Module['asmExports'] = {};
167167
});
168-
for (auto* curr : module->exports) {
168+
for (auto& curr : module->exports) {
169169
EM_ASM_({
170170
var name = Pointer_stringify($0);
171171
Module['asmExports'][name] = function() {
@@ -177,7 +177,7 @@ extern "C" void EMSCRIPTEN_KEEPALIVE instantiate() {
177177
}
178178

179179
// verify imports are provided
180-
for (auto* import : module->imports) {
180+
for (auto& import : module->imports) {
181181
EM_ASM_({
182182
var mod = Pointer_stringify($0);
183183
var base = Pointer_stringify($1);
@@ -199,7 +199,7 @@ extern "C" void EMSCRIPTEN_KEEPALIVE instantiate() {
199199
var source = Module['HEAP8'].subarray($1, $1 + $2);
200200
var target = new Int8Array(Module['outside']['newBuffer']);
201201
target.set(source, $0);
202-
}, segment.offset, segment.data, segment.size);
202+
}, segment.offset, &segment.data[0], segment.data.size());
203203
}
204204
}
205205

@@ -241,11 +241,13 @@ extern "C" void EMSCRIPTEN_KEEPALIVE instantiate() {
241241

242242
Literal load(Load* load, size_t addr) override {
243243
if (load->align < load->bytes || (addr & (load->bytes-1))) {
244+
int64_t out64;
244245
double ret = EM_ASM_DOUBLE({
245246
var addr = $0;
246247
var bytes = $1;
247248
var isFloat = $2;
248249
var isSigned = $3;
250+
var out64 = $4;
249251
var save0 = HEAP32[0];
250252
var save1 = HEAP32[1];
251253
for (var i = 0; i < bytes; i++) {
@@ -255,23 +257,24 @@ extern "C" void EMSCRIPTEN_KEEPALIVE instantiate() {
255257
if (!isFloat) {
256258
if (bytes === 1) ret = isSigned ? HEAP8[0] : HEAPU8[0];
257259
else if (bytes === 2) ret = isSigned ? HEAP16[0] : HEAPU16[0];
258-
else if (bytes === 4 || bytes === 8) ret = isSigned ? HEAP32[0] : HEAPU32[0]; // if i64, return low 32 bits here
259-
else abort();
260+
else if (bytes === 4) ret = isSigned ? HEAP32[0] : HEAPU32[0];
261+
else if (bytes === 8) {
262+
for (var i = 0; i < bytes; i++) {
263+
HEAPU8[out64 + i] = HEAPU8[i];
264+
}
265+
} else abort();
260266
} else {
261267
if (bytes === 4) ret = HEAPF32[0];
262268
else if (bytes === 8) ret = HEAPF64[0];
263269
else abort();
264270
}
265271
HEAP32[0] = save0; HEAP32[1] = save1;
266272
return ret;
267-
}, addr, load->bytes, isWasmTypeFloat(load->type), load->signed_);
273+
}, addr, load->bytes, isWasmTypeFloat(load->type), load->signed_, &out64);
268274
if (!isWasmTypeFloat(load->type)) {
269275
if (load->type == i64) {
270276
if (load->bytes == 8) {
271-
int32_t high = EM_ASM_INT_V({
272-
return HEAPU32[1];
273-
});
274-
return Literal(int64_t(int32_t(ret)) | (int64_t(int32_t(high)) << 32));
277+
return Literal(out64);
275278
} else {
276279
if (load->signed_) {
277280
return Literal(int64_t(int32_t(ret)));

‎test/wasm_backend/i64_load.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdint.h>
2+
#include <stdio.h>
3+
4+
struct S1 { unsigned lo:32; unsigned mid:2; unsigned hi:30; };
5+
6+
static struct S1 g_68 = { -1, 0, 0xbbe }; // 0xbbe = 3006
7+
8+
extern "C" void emscripten_autodebug_i32(int32_t x, int32_t y);
9+
10+
int main() {
11+
emscripten_autodebug_i32(0, g_68.lo);
12+
emscripten_autodebug_i32(1, g_68.mid);
13+
emscripten_autodebug_i32(2, g_68.hi);
14+
return 0;
15+
}

‎test/wasm_backend/i64_load.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
AD:0,-1
2+
AD:1,0
3+
AD:2,3006
4+

0 commit comments

Comments
 (0)