Skip to content

Commit 708886f

Browse files
authored
Clean up and optimize sbrk() (#12222)
After we no longer include malloc/free by default (#12213) we can remove the hack in sbrk which avoided using static memory. This gives some nice code size improvements. However this can't be as simple as we'd hope because of dynamic linking plus SAFE_HEAP. In that mode we may try to use sbrk_val before the dynamic linker has set it up. This is because SAFE_HEAP is not aware of dynamic linking - it just instruments all loads and stores, period, including ones that happen during dynamic linking startup. To avoid ordering issues there, initialize sbrk_val manually.
1 parent a867634 commit 708886f

File tree

5 files changed

+29
-37
lines changed

5 files changed

+29
-37
lines changed

system/lib/sbrk.c

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,17 @@
2929
#define SET_ERRNO()
3030
#endif
3131

32-
// FIXME just use = &sbrk_val here. That is simpler, but currently it has a
33-
// code size cost for tiny programs that don't use malloc. We link in malloc by
34-
// default, and depend on metadce to remove it when not used, which we can do,
35-
// but a static assignment here would mean a nonzero value in a memory segment,
36-
// which metadce cannot remove (it can remove code, but not data in segments).
37-
// The current code allows tiny programs without malloc to have no data segments
38-
// at all, while programs that use malloc may suffer a few more bytes of code
39-
// size, but that's negligible compared to malloc itself.
40-
// TODO: when we stop including malloc by default we can use the simpler way.
41-
static intptr_t sbrk_val = 0;
32+
extern size_t __heap_base;
33+
34+
static intptr_t sbrk_val = (intptr_t)&__heap_base;
4235

4336
intptr_t* emscripten_get_sbrk_ptr() {
44-
extern size_t __heap_base;
45-
intptr_t* sbrk_ptr = &sbrk_val;
46-
#if __EMSCRIPTEN_PTHREADS__
47-
if (__c11_atomic_load((_Atomic(intptr_t)*)sbrk_ptr, __ATOMIC_SEQ_CST) == 0) {
48-
__c11_atomic_store((_Atomic(intptr_t)*)sbrk_ptr, (intptr_t)&__heap_base, __ATOMIC_SEQ_CST);
49-
}
50-
#else
37+
#ifdef __PIC__
38+
// In relocatable code we may call emscripten_get_sbrk_ptr() during startup,
39+
// potentially *before* the setup of the dynamically-linked __heap_base, when
40+
// using SAFE_HEAP. (SAFE_HEAP instruments *all* memory accesses, so even the
41+
// code doing dynamic linking itself ends up instrumented, which is why we can
42+
// get such an instrumented call before sbrk_val has its proper value.)
5143
if (sbrk_val == 0) {
5244
sbrk_val = (intptr_t)&__heap_base;
5345
}

tests/code_size/hello_webgl2_wasm.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"a.html.gz": 377,
44
"a.js": 5035,
55
"a.js.gz": 2395,
6-
"a.wasm": 10910,
7-
"a.wasm.gz": 6928,
8-
"total": 16508,
9-
"total_gz": 9700
6+
"a.wasm": 10891,
7+
"a.wasm.gz": 6915,
8+
"total": 16489,
9+
"total_gz": 9687
1010
}
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.html": 588,
33
"a.html.gz": 386,
4-
"a.js": 22128,
5-
"a.js.gz": 8459,
6-
"a.mem": 3168,
7-
"a.mem.gz": 2711,
8-
"total": 25884,
9-
"total_gz": 11556
4+
"a.js": 22091,
5+
"a.js.gz": 8449,
6+
"a.mem": 3171,
7+
"a.mem.gz": 2715,
8+
"total": 25850,
9+
"total_gz": 11550
1010
}

tests/code_size/hello_webgl_wasm.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"a.html.gz": 377,
44
"a.js": 4519,
55
"a.js.gz": 2219,
6-
"a.wasm": 10910,
7-
"a.wasm.gz": 6928,
8-
"total": 15992,
9-
"total_gz": 9524
6+
"a.wasm": 10891,
7+
"a.wasm.gz": 6915,
8+
"total": 15973,
9+
"total_gz": 9511
1010
}
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.html": 588,
33
"a.html.gz": 386,
4-
"a.js": 21617,
5-
"a.js.gz": 8297,
6-
"a.mem": 3168,
7-
"a.mem.gz": 2711,
8-
"total": 25373,
9-
"total_gz": 11394
4+
"a.js": 21580,
5+
"a.js.gz": 8289,
6+
"a.mem": 3171,
7+
"a.mem.gz": 2715,
8+
"total": 25339,
9+
"total_gz": 11390
1010
}

0 commit comments

Comments
 (0)