diff --git a/ChangeLog.md b/ChangeLog.md index e97b4adad87f5..c89d00a8e8500 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -20,6 +20,12 @@ See docs/process.md for more on how version tagging works. 6.0.3 (in development) ---------------------- +- The `GROWABLE_ARRAYBUFFERS` setting default was reverted back to 0 (it was + changed to 1 in 6.0.2) because `TextDecoder.decode()` rejects views of + resizable ArrayBuffers in browsers, breaking `UTF8ToString` in programs + built with `ALLOW_MEMORY_GROWTH`. String decoding now copies the data when + the heap buffer is resizable, so the setting can still be enabled + explicitly. (#27241) - Added support for compiling FMA intrinsics. All 32 FMA intrinsics are supported, with 256-bit variants emulated via two 128-bit operations. Pass ``-msimd128 -mfma`` to enable. With ``-mrelaxed-simd -mfma``, Wasm relaxed diff --git a/package-lock.json b/package-lock.json index 182028c9f802a..a0189d82e9f37 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "emscripten", + "name": "issue-438", "lockfileVersion": 3, "requires": true, "packages": { diff --git a/site/source/docs/tools_reference/settings_reference.rst b/site/source/docs/tools_reference/settings_reference.rst index ec12708d8ba48..cca2746b2f0ab 100644 --- a/site/source/docs/tools_reference/settings_reference.rst +++ b/site/source/docs/tools_reference/settings_reference.rst @@ -3435,13 +3435,13 @@ GROWABLE_ARRAYBUFFERS Enable support for growable views of Wasm memory. This is a recent Web platform feature that can make growing the Wasm memory more efficient, especially in multi-threaded builds. -The default setting of 1 will auto-detect the presence of this API and use +Setting this to 1 will auto-detect the presence of this API and use it when available. Setting this to 2 will unconditionally require it. This is the only way to completely remove the overhead of growable memory + pthreads. This settings does nothing unless ALLOW_MEMORY_GROWTH is set. -Default value: 1 +Default value: 0 .. _cross_origin: diff --git a/src/parseTools.mjs b/src/parseTools.mjs index 61e4d20565de1..c442767d48873 100644 --- a/src/parseTools.mjs +++ b/src/parseTools.mjs @@ -1078,27 +1078,45 @@ function runtimeKeepalivePop() { return 'runtimeKeepalivePop();'; } -// Some web functions like TextDecoder.decode() may not work with a view of a -// SharedArrayBuffer, see https://github.com/whatwg/encoding/issues/172 -// To avoid that, this function allows obtaining an unshared copy of an -// ArrayBuffer. +// Some web functions like TextDecoder.decode() do not work with a view of a +// SharedArrayBuffer (see https://github.com/whatwg/encoding/issues/172) or of +// a resizable ArrayBuffer (see +// https://github.com/emscripten-core/emscripten/issues/27241). +// To avoid that, this function allows obtaining a copy in those cases. function getUnsharedTextDecoderView(heap, start, end) { - const shared = `${heap}.slice(${start}, ${end})`; - const unshared = `${heap}.subarray(${start}, ${end})`; + const copy = `${heap}.slice(${start}, ${end})`; + const view = `${heap}.subarray(${start}, ${end})`; - // No need to worry about this in non-shared memory builds - if (!SHARED_MEMORY) return unshared; + // The heap can be backed by a resizable ArrayBuffer in non-shared memory + // builds with memory growth enabled. + const maybeResizable = !SHARED_MEMORY && ALLOW_MEMORY_GROWTH && GROWABLE_ARRAYBUFFERS; - // If asked to get an unshared view to what we know will be a shared view, or - // if in -Oz, then unconditionally do a .slice() for smallest code size. - // This is guaranteed to work but could be slower since it performs a copy. - if (SHRINK_LEVEL == 2 || heap.startsWith('HEAP')) return shared; + // No need to worry about this in builds where the buffer can be neither + // shared nor resizable. + if (!SHARED_MEMORY && !maybeResizable) return view; - // Otherwise, generate a runtime type check: must do a .slice() if looking at - // a SAB, or can use .subarray() otherwise. Note: We compare with - // `ArrayBuffer` here to avoid referencing `SharedArrayBuffer` which could be - // undefined. - return `${heap}.buffer instanceof ArrayBuffer ? ${unshared} : ${shared}`; + // If in -Oz, then unconditionally do a .slice() for smallest code size. + // This is guaranteed to work but could be slower since it performs a copy. + if (SHRINK_LEVEL == 2) return copy; + + if (SHARED_MEMORY) { + // If asked to get an unshared view to what we know will be a shared view, + // then unconditionally do a .slice(). + if (heap.startsWith('HEAP')) return copy; + + // Otherwise, generate a runtime type check: must do a .slice() if looking + // at a SAB, or can use .subarray() otherwise. Note: We compare with + // `ArrayBuffer` here to avoid referencing `SharedArrayBuffer` which could + // be undefined. + return `${heap}.buffer instanceof ArrayBuffer ? ${view} : ${copy}`; + } + + // With GROWABLE_ARRAYBUFFERS == 2 the heap is always resizable; with + // GROWABLE_ARRAYBUFFERS == 1 resizability is feature-detected at runtime, + // and non-heap views passed to UTF8ArrayToString may not be resizable at + // all, so generate a runtime check in those cases. + if (GROWABLE_ARRAYBUFFERS == 2 && heap.startsWith('HEAP')) return copy; + return `${heap}.buffer.resizable ? ${copy} : ${view}`; } function getEntryFunction() { diff --git a/src/settings.js b/src/settings.js index eca9ba43602a7..24c88a7be4749 100644 --- a/src/settings.js +++ b/src/settings.js @@ -2276,13 +2276,13 @@ var JS_BASE64_API = false; // Enable support for growable views of Wasm memory. This is a recent Web // platform feature that can make growing the Wasm memory more efficient, // especially in multi-threaded builds. -// The default setting of 1 will auto-detect the presence of this API and use +// Setting this to 1 will auto-detect the presence of this API and use // it when available. // Setting this to 2 will unconditionally require it. This is the only way // to completely remove the overhead of growable memory + pthreads. // This settings does nothing unless ALLOW_MEMORY_GROWTH is set. // [link] -var GROWABLE_ARRAYBUFFERS = 1; +var GROWABLE_ARRAYBUFFERS = 0; // If the emscripten-generated program is hosted on separate origin then // starting new pthread worker can violate CSP rules. Enabling diff --git a/test/codesize/test_codesize_mem_O3_grow.json b/test/codesize/test_codesize_mem_O3_grow.json index 47a146c9c54f5..bf38c5463fb01 100644 --- a/test/codesize/test_codesize_mem_O3_grow.json +++ b/test/codesize/test_codesize_mem_O3_grow.json @@ -1,10 +1,10 @@ { - "a.out.js": 4386, - "a.out.js.gz": 2154, + "a.out.js": 4339, + "a.out.js.gz": 2132, "a.out.nodebug.wasm": 5261, "a.out.nodebug.wasm.gz": 2419, - "total": 9647, - "total_gz": 4573, + "total": 9600, + "total_gz": 4551, "sent": [ "a (emscripten_resize_heap)" ], diff --git a/test/codesize/test_codesize_mem_O3_grow_standalone.json b/test/codesize/test_codesize_mem_O3_grow_standalone.json index c3689358c5a28..a9a233b4aae65 100644 --- a/test/codesize/test_codesize_mem_O3_grow_standalone.json +++ b/test/codesize/test_codesize_mem_O3_grow_standalone.json @@ -1,10 +1,10 @@ { - "a.out.js": 3852, - "a.out.js.gz": 1896, + "a.out.js": 3805, + "a.out.js.gz": 1869, "a.out.nodebug.wasm": 5641, "a.out.nodebug.wasm.gz": 2659, - "total": 9493, - "total_gz": 4555, + "total": 9446, + "total_gz": 4528, "sent": [ "args_get", "args_sizes_get", diff --git a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json index 371d9194c76c6..75bf66aecb973 100644 --- a/test/codesize/test_codesize_minimal_pthreads_memgrowth.json +++ b/test/codesize/test_codesize_minimal_pthreads_memgrowth.json @@ -1,10 +1,10 @@ { - "a.out.js": 7543, - "a.out.js.gz": 3705, + "a.out.js": 7372, + "a.out.js.gz": 3635, "a.out.nodebug.wasm": 19064, "a.out.nodebug.wasm.gz": 8804, - "total": 26607, - "total_gz": 12509, + "total": 26436, + "total_gz": 12439, "sent": [ "a (memory)", "b (exit)", diff --git a/test/test_other.py b/test/test_other.py index 6c8c26e4906d7..506023a4f8cc0 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -15492,6 +15492,40 @@ def test_TextDecoder_invalid(self): expected = '#error "TEXTDECODER must be either 1 or 2"' self.assert_fail([EMCC, test_file('hello_world.c'), '-sTEXTDECODER=3'], expected) + # Verify that strings can be decoded when the heap is backed by a resizable + # ArrayBuffer (ALLOW_MEMORY_GROWTH + GROWABLE_ARRAYBUFFERS). + # TextDecoder.decode() rejects views of resizable ArrayBuffers, so we must + # pass it a copy of the data instead. Node's TextDecoder happens to accept + # such views, so emulate the browser behaviour in a --pre-js. + # See https://github.com/emscripten-core/emscripten/issues/27241 + @requires_node_26 + @parameterized({ + '': ([],), + 'textdecoder_2': (['-sTEXTDECODER=2'],), + }) + def test_TextDecoder_resizable_buffer(self, args): + create_file('pre.js', ''' + var realDecode = TextDecoder.prototype.decode; + TextDecoder.prototype.decode = function(data) { + if (data && data.buffer && (data.buffer.resizable || data.buffer.growable)) { + throw new TypeError('The provided ArrayBuffer value must not be resizable'); + } + return realDecode.call(this, data); + }; + ''') + create_file('main.c', r''' + #include + + int main() { + // Long enough (> 16 bytes) that UTF8ToString on the string takes the + // TextDecoder path. + emscripten_out("the quick brown fox jumps over the lazy dog"); + return 0; + } + ''') + self.do_runf('main.c', 'the quick brown fox jumps over the lazy dog', + cflags=['--pre-js=pre.js', '-sALLOW_MEMORY_GROWTH', '-sGROWABLE_ARRAYBUFFERS=1'] + args) + def test_reallocarray(self): self.do_other_test('test_reallocarray.c')