Skip to content

Commit fb5b26f

Browse files
authored
Avoiding closing over HEAPU32/HEAPU64 in emval.js (#17556)
Its not generally save to close over these variables because they get re-bound on memory growth. See https://github.com/emscripten-core/emscripten/pull/17293/files/6cd449dba7419e3393962a7c0a6ee6d6d1f8a295#r933701376
1 parent 7c1d7ea commit fb5b26f

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

src/embind/emval.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,15 @@ var LibraryEmVal = {
201201
argsList += (i!==0?", ":"")+"arg"+i; // 'arg0, arg1, ..., argn'
202202
}
203203

204+
// The body of the generated function does not have access to enclosing
205+
// scope where HEAPU64/HEAPU32/etc are defined, and we cannot pass them
206+
// directly as arguments (like we do the Module object) since memory
207+
// growth can cause them to be re-bound.
208+
var getMemory = () => {{{ MEMORY64 ? "HEAPU64" : "HEAPU32" }}};
209+
204210
var functionBody =
205-
"return function emval_allocator_"+argCount+"(constructor, argTypes, args) {\n";
211+
"return function emval_allocator_"+argCount+"(constructor, argTypes, args) {\n" +
212+
" var {{{ MEMORY64 ? 'HEAPU64' : 'HEAPU32' }}} = getMemory();\n";
206213

207214
for (var i = 0; i < argCount; ++i) {
208215
functionBody +=
@@ -217,8 +224,8 @@ var LibraryEmVal = {
217224
"}\n";
218225

219226
/*jshint evil:true*/
220-
return (new Function("requireRegisteredType", "Module", "valueToHandle", "{{{ MEMORY64 ? "HEAPU64" : "HEAPU32" }}}" , functionBody))(
221-
requireRegisteredType, Module, Emval.toHandle, {{{ MEMORY64 ? "HEAPU64" : "HEAPU32" }}});
227+
return (new Function("requireRegisteredType", "Module", "valueToHandle", "getMemory" , functionBody))(
228+
requireRegisteredType, Module, Emval.toHandle, getMemory);
222229
#endif
223230
},
224231

test/embind/embind_test.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
#include <malloc.h>
88
#include <functional>
99
#include <emscripten/bind.h>
10+
#include <emscripten/heap.h>
11+
#include <emscripten/em_asm.h>
1012

1113
using namespace emscripten;
1214

@@ -192,8 +194,14 @@ std::u32string get_literal_u32string() {
192194

193195
void force_memory_growth() {
194196
val module = val::global("Module");
195-
std::size_t heap_size = module["HEAPU8"]["byteLength"].as<size_t>();
196-
free(malloc(heap_size + 1));
197+
std::size_t old_size = emscripten_get_heap_size();
198+
EM_ASM({"globalThis.oldheap = HEAPU8;"});
199+
assert(val::global("oldheap")["byteLength"].as<size_t>() == old_size);
200+
emscripten_resize_heap(old_size + EMSCRIPTEN_PAGE_SIZE);
201+
assert(emscripten_get_heap_size() > old_size);
202+
// global HEAPU8 should now be rebound, and our oldheap should be detached
203+
assert(module["HEAPU8"]["byteLength"].as<size_t>() > old_size);
204+
assert(val::global("oldheap")["byteLength"].as<size_t>() == 0);
197205
}
198206

199207
std::string emval_test_take_and_return_const_char_star(const char* str) {

0 commit comments

Comments
 (0)