From 2f59c61a038018084eb1dfe07276ef074191c127 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Thu, 19 Jan 2023 12:49:02 +0900 Subject: [PATCH 1/7] dlmalloc: require __heap_end This commit effectively drops the support of older wasm-ld. (LLVM <15.0.7) We have two relevant use cases: * `memory.grow` use outside of malloc (eg. used by polyfill preview1 binaries) * `--init-memory` to somehow preallocate heap (eg. avoid dynamic allocations, especially on small environments) While https://github.com/WebAssembly/wasi-libc/pull/377 fixed the former, it broke the latter if you are using an older LLVM, which doesn't provide the `__heap_end` symbol, to link your module. As we couldn't come up with a solution which satisfies all parties, this commit simply makes it require new enough LLVM which provides `__heap_end`. After all, a link-time failure is more friendly to users than failing later in a subtle way. --- dlmalloc/src/malloc.c | 25 ++++++------------- .../wasm32-wasi-threads/undefined-symbols.txt | 1 + expected/wasm32-wasi/undefined-symbols.txt | 1 + 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/dlmalloc/src/malloc.c b/dlmalloc/src/malloc.c index c9c8ea6ee..ce55162ba 100644 --- a/dlmalloc/src/malloc.c +++ b/dlmalloc/src/malloc.c @@ -5215,7 +5215,7 @@ static void internal_inspect_all(mstate m, /* Symbol marking the end of data, bss and explicit stack, provided by wasm-ld. */ extern char __heap_base; -extern char __heap_end __attribute__((__weak__)); +extern char __heap_end; /* Initialize the initial state of dlmalloc to be able to use free memory between __heap_base and initial. */ static void try_init_allocator(void) { @@ -5227,23 +5227,14 @@ static void try_init_allocator(void) { char *base = &__heap_base; // Try to use the linker pseudo-symbol `__heap_end` for the initial size of - // the heap, but if that's not defined due to LLVM being too old perhaps then - // round up `base` to the nearest `PAGESIZE`. The initial size of linear - // memory will be at least the heap base to this page boundary, and it's then - // assumed that the initial linear memory image was truncated at that point. - // While this reflects the default behavior of `wasm-ld` it is also possible - // for users to craft larger linear memories by passing options to extend - // beyond this threshold. In this situation the memory will not be used for - // dlmalloc. - // - // Note that `sbrk(0)`, or in dlmalloc-ese `CALL_MORECORE(0)`, is specifically - // not used here. That captures the current size of the heap but is only - // correct if the we're the first to try to grow the heap. If the heap has - // grown elsewhere, such as a different allocator in place, then this would - // incorrectly claim such memroy as our own. + // the heap. char *end = &__heap_end; - if (end == NULL) - end = (char*) page_align((size_t) base); + if (end == NULL) { + // This can happen when 1. you are using an old wasm-ld which doesn't + // provide `__heap_end` and 2. something (other libraries or maybe + // your app?) provide a weak reference to `__heap_end`. + return; + } size_t initial_heap_size = end - base; /* Check that initial heap is long enough to serve a minimal allocation request. */ diff --git a/expected/wasm32-wasi-threads/undefined-symbols.txt b/expected/wasm32-wasi-threads/undefined-symbols.txt index a68515194..c04b5f703 100644 --- a/expected/wasm32-wasi-threads/undefined-symbols.txt +++ b/expected/wasm32-wasi-threads/undefined-symbols.txt @@ -13,6 +13,7 @@ __getf2 __global_base __gttf2 __heap_base +__heap_end __imported_wasi_snapshot_preview1_args_get __imported_wasi_snapshot_preview1_args_sizes_get __imported_wasi_snapshot_preview1_clock_res_get diff --git a/expected/wasm32-wasi/undefined-symbols.txt b/expected/wasm32-wasi/undefined-symbols.txt index b9cdb3432..6d3b2b7d6 100644 --- a/expected/wasm32-wasi/undefined-symbols.txt +++ b/expected/wasm32-wasi/undefined-symbols.txt @@ -11,6 +11,7 @@ __floatunsitf __getf2 __gttf2 __heap_base +__heap_end __imported_wasi_snapshot_preview1_args_get __imported_wasi_snapshot_preview1_args_sizes_get __imported_wasi_snapshot_preview1_clock_res_get From f5f2d770fdf0a394053871ef2ff4dcfec7c2b4ce Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 17 Feb 2023 15:21:21 +0900 Subject: [PATCH 2/7] dlmalloc: add a comment --- dlmalloc/src/malloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlmalloc/src/malloc.c b/dlmalloc/src/malloc.c index ce55162ba..803fa6d13 100644 --- a/dlmalloc/src/malloc.c +++ b/dlmalloc/src/malloc.c @@ -5233,6 +5233,8 @@ static void try_init_allocator(void) { // This can happen when 1. you are using an old wasm-ld which doesn't // provide `__heap_end` and 2. something (other libraries or maybe // your app?) provide a weak reference to `__heap_end`. + // + // Note: This is a linker bug: https://github.com/llvm/llvm-project/issues/60829 return; } size_t initial_heap_size = end - base; From 1cc8c8fef696b6f47afebcfa068b880f62596d4f Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 17 Feb 2023 13:26:22 +0900 Subject: [PATCH 3/7] CI: Bump llvm version --- .github/workflows/main.yml | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 83c2751fd..d9b33af2a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -8,8 +8,26 @@ jobs: strategy: matrix: os: [ubuntu-latest, macos-latest, windows-latest] - # oldest and newest supported LLVM version - clang_version: [10.0.0, 14.0.0] + clang_version: [10.0.0] + # use different LLVM versions among oses because of the lack of + # official assets on github. + include: + - os: ubuntu-latest + clang_version: 10.0.0 + llvm_asset_suffix: x86_64-linux-gnu-ubuntu-18.04 + - os: macos-latest + clang_version: 10.0.0 + llvm_asset_suffix: x86_64-apple-darwin + - os: windows-latest + clang_version: 10.0.0 + - os: ubuntu-latest + clang_version: 16.0.0 + llvm_asset_suffix: x86_64-linux-gnu-ubuntu-18.04 + - os: macos-latest + clang_version: 15.0.7 + llvm_asset_suffix: x86_64-apple-darwin21.0 + - os: windows-latest + clang_version: 16.0.0 steps: - uses: actions/checkout@v1 with: @@ -44,8 +62,8 @@ jobs: - name: Install LLVM tools (MacOS) shell: bash run: | - curl -sSfL https://github.com/llvm/llvm-project/releases/download/llvmorg-${{ matrix.clang_version }}/clang+llvm-${{ matrix.clang_version }}-x86_64-apple-darwin.tar.xz | tar xJf - - export CLANG_DIR=`pwd`/clang+llvm-${{ matrix.clang_version }}-x86_64-apple-darwin/bin + curl -sSfL https://github.com/llvm/llvm-project/releases/download/llvmorg-${{ matrix.clang_version }}/clang+llvm-${{ matrix.clang_version }}-${{ matrix.llvm_asset_suffix }}.tar.xz | tar xJf - + export CLANG_DIR=`pwd`/clang+llvm-${{ matrix.clang_version }}-${{ matrix.llvm_asset_suffix }}/bin echo "$CLANG_DIR" >> $GITHUB_PATH echo "CC=$CLANG_DIR/clang" >> $GITHUB_ENV echo "AR=$CLANG_DIR/llvm-ar" >> $GITHUB_ENV @@ -55,8 +73,8 @@ jobs: - name: Install LLVM tools (Linux) shell: bash run: | - curl -sSfL https://github.com/llvm/llvm-project/releases/download/llvmorg-${{ matrix.clang_version }}/clang+llvm-${{ matrix.clang_version }}-x86_64-linux-gnu-ubuntu-18.04.tar.xz | tar xJf - - export CLANG_DIR=`pwd`/clang+llvm-${{ matrix.clang_version }}-x86_64-linux-gnu-ubuntu-18.04/bin + curl -sSfL https://github.com/llvm/llvm-project/releases/download/llvmorg-${{ matrix.clang_version }}/clang+llvm-${{ matrix.clang_version }}-${{ matrix.llvm_asset_suffix }}.tar.xz | tar xJf - + export CLANG_DIR=`pwd`/clang+llvm-${{ matrix.clang_version }}-${{ matrix.llvm_asset_suffix }}/bin echo "$CLANG_DIR" >> $GITHUB_PATH echo "CLANG_DIR=$CLANG_DIR" >> $GITHUB_ENV echo "CC=$CLANG_DIR/clang" >> $GITHUB_ENV @@ -82,6 +100,8 @@ jobs: make test # The older version of Clang does not provide the expected symbol for the # test entrypoints: `undefined symbol: __main_argc_argv`. + # The older (<15.0.7) version of wasm-ld does not provide `__heap_end`, + # which is required by our malloc implementation. if: matrix.os == 'ubuntu-latest' && matrix.clang_version != '10.0.0' - uses: actions/upload-artifact@v1 From adc74d79c03ad803684385a75c6a13dc66c8f9e9 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 22 Mar 2023 09:41:13 +0900 Subject: [PATCH 4/7] Deal with the resource dir change cf. https://reviews.llvm.org/D125860 --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d9b33af2a..eb9b59f83 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -94,7 +94,7 @@ jobs: run: | cd test make download - export WASI_DIR=$(realpath $CLANG_DIR/../lib/clang/${{ matrix.clang_version }}/lib/wasi/) + export WASI_DIR=$(realpath $($CLANG_DIR/clang -print-resource-dir)/lib/wasi/) mkdir -p $WASI_DIR cp download/lib/wasi/libclang_rt.builtins-wasm32.a $WASI_DIR make test From 01cac38c49cf02a3e2fa4644a81c610b8e25210e Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 19 Apr 2023 10:49:25 +0900 Subject: [PATCH 5/7] dlmalloc: a comment --- dlmalloc/src/malloc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlmalloc/src/malloc.c b/dlmalloc/src/malloc.c index 803fa6d13..e5538bf19 100644 --- a/dlmalloc/src/malloc.c +++ b/dlmalloc/src/malloc.c @@ -5231,8 +5231,10 @@ static void try_init_allocator(void) { char *end = &__heap_end; if (end == NULL) { // This can happen when 1. you are using an old wasm-ld which doesn't - // provide `__heap_end` and 2. something (other libraries or maybe - // your app?) provide a weak reference to `__heap_end`. + // provide `__heap_end` (< 15.0.7) and 2. something (other libraries + // or maybe your app?) includes a weak reference to `__heap_end` and + // 3. the weak reference is found by the linker before this strong + // reference. // // Note: This is a linker bug: https://github.com/llvm/llvm-project/issues/60829 return; From fd9a8ce3fde4a6d212eb0c4d72e8710203acde73 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 19 Apr 2023 10:54:25 +0900 Subject: [PATCH 6/7] dlmalloc: ABORT on the known linker bug --- dlmalloc/src/malloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlmalloc/src/malloc.c b/dlmalloc/src/malloc.c index e5538bf19..d992ef1e7 100644 --- a/dlmalloc/src/malloc.c +++ b/dlmalloc/src/malloc.c @@ -5237,7 +5237,7 @@ static void try_init_allocator(void) { // reference. // // Note: This is a linker bug: https://github.com/llvm/llvm-project/issues/60829 - return; + ABORT; } size_t initial_heap_size = end - base; From aa35d19494b38d605f3990e36c6e0cec5ebd9945 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 19 Apr 2023 11:51:16 +0900 Subject: [PATCH 7/7] dlmalloc: avoid LLVM optimizer eliminating the __heap_end check --- dlmalloc/src/malloc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlmalloc/src/malloc.c b/dlmalloc/src/malloc.c index d992ef1e7..7794fc81b 100644 --- a/dlmalloc/src/malloc.c +++ b/dlmalloc/src/malloc.c @@ -5229,15 +5229,15 @@ static void try_init_allocator(void) { // Try to use the linker pseudo-symbol `__heap_end` for the initial size of // the heap. char *end = &__heap_end; - if (end == NULL) { - // This can happen when 1. you are using an old wasm-ld which doesn't + if (end < base) { + // "end" can be NULL when 1. you are using an old wasm-ld which doesn't // provide `__heap_end` (< 15.0.7) and 2. something (other libraries // or maybe your app?) includes a weak reference to `__heap_end` and // 3. the weak reference is found by the linker before this strong // reference. // // Note: This is a linker bug: https://github.com/llvm/llvm-project/issues/60829 - ABORT; + __builtin_trap(); } size_t initial_heap_size = end - base;