[lld][WebAssembly] Align __heap_base

[aykevl]

Summary

__heap_base was not aligned. In practice, it will often be aligned simply because it follows the stack, but when the stack is placed at the beginning (with the --stack-first option), the __heap_base might be unaligned. It could even be byte-aligned.

At least wasi-libc appears to expect that __heap_base is aligned: https://github.com/WebAssembly/wasi-libc/blob/659ff414560721b1660a19685110e484a081c3d4/dlmalloc/src/malloc.c#L5224

While WebAssembly itself does not appear to require any alignment for memory accesses, it is sometimes required when sharing a pointer externally. For example, WASI might expect alignment up to 8: https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md#-timestamp-u64

This issue got introduced with the addition of the --stack-first flag: https://reviews.llvm.org/D46141
I suspect the lack of alignment wasn't intentional here.

[sbc100]

Nice! Thanks.

lld/wasm/Writer.cpp

46

I think it would make sense to use 16 here since the default ABI has a max_align_t of 16.

Maybe just call this heapAlignment?

[aykevl]

• change alignment to 16
• change heapBaseAlignment to heapAlignment

[aykevl]

Thank you for the quick review! I'll commit this soon-ish unless someone has comments on the patch.

lld/wasm/Writer.cpp

46

Sounds good! I've updated the patch according to your suggestions.

[sunfish]

Looks good to me too!

lld/test/wasm/Inputs/stack-first.s

+  .globl  _start
+_start:
+  .functype _start () -> ()
+  end_function
+
+  .globl     someByte
+  .type      someByte,@object
+  .section  .data.someByte,"",@
+someByte:
+  .int8 42
+  .size someByte, 1

lld/test/wasm/stack-first.test

 ; Test that the --stack-first option places the stack at the start of linear
 ; memory.  In this case the --stack-first option is being passed along with a
 ; stack size of 512.  This means (since the stack grows down) the stack pointer
 ; global should be initialized to 512.
+; Also test that __heap_base is still aligned with the --stack-first option.
 
-RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %t.o
-RUN: wasm-ld -z stack-size=512 --stack-first --export=__data_end --export=__heap_base -o %t.wasm %t.o
+RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/stack-first.s -o %t.o
+RUN: wasm-ld -z stack-size=512 --stack-first --export=__data_end --export=__heap_base --export=someByte -o %t.wasm %t.o
 RUN: obj2yaml %t.wasm | FileCheck %s
 
 CHECK:        - Type:            GLOBAL
 CHECK-NEXT:     Globals:         
 CHECK-NEXT:       - Index:           0
 CHECK-NEXT:         Type:            I32
 CHECK-NEXT:         Mutable:         true
 CHECK-NEXT:         InitExpr:        
 CHECK-NEXT:           Opcode:          I32_CONST
 CHECK-NEXT:           Value:           512
 CHECK-NEXT:       - Index:           1
 CHECK-NEXT:         Type:            I32
 CHECK-NEXT:         Mutable:         false
 CHECK-NEXT:         InitExpr:        
 CHECK-NEXT:           Opcode:          I32_CONST
 CHECK-NEXT:           Value:           512
 CHECK-NEXT:       - Index:           2
 CHECK-NEXT:         Type:            I32
 CHECK-NEXT:         Mutable:         false
 CHECK-NEXT:         InitExpr:        
 CHECK-NEXT:           Opcode:          I32_CONST
-CHECK-NEXT:           Value:           512
+CHECK-NEXT:           Value:           513
+CHECK-NEXT:       - Index:           3
+CHECK-NEXT:         Type:            I32
+CHECK-NEXT:         Mutable:         false
+CHECK-NEXT:         InitExpr:        
+CHECK-NEXT:           Opcode:          I32_CONST
+CHECK-NEXT:           Value:           528
 CHECK-NEXT:   - Type:            EXPORT
 CHECK-NEXT:     Exports:         
 CHECK-NEXT:       - Name:            memory
 CHECK-NEXT:         Kind:            MEMORY
 CHECK-NEXT:         Index:           0
 CHECK-NEXT:       - Name:            _start
 CHECK-NEXT:         Kind:            FUNCTION
 CHECK-NEXT:         Index:           0
-CHECK-NEXT:       - Name:            __data_end
+CHECK-NEXT:       - Name:            someByte
 CHECK-NEXT:         Kind:            GLOBAL
 CHECK-NEXT:         Index:           1
-CHECK-NEXT:       - Name:            __heap_base
+CHECK-NEXT:       - Name:            __data_end
 CHECK-NEXT:         Kind:            GLOBAL
 CHECK-NEXT:         Index:           2
+CHECK-NEXT:       - Name:            __heap_base
+CHECK-NEXT:         Kind:            GLOBAL
+CHECK-NEXT:         Index:           3

lld/wasm/Writer.cpp

 #define DEBUG_TYPE "lld"
 
 using namespace llvm;
 using namespace llvm::wasm;
 
 namespace lld {
 namespace wasm {
 static constexpr int stackAlignment = 16;
+static constexpr int heapAlignment = 16;
 
 namespace {
 
 // The writer writes a SymbolTable result to a file.
 class Writer {
 public:
   void run();
 
void Writer::layoutMemory() {
   log("mem: static data = " + Twine(staticDataSize));
   if (config->isPic)
     out.dylinkSec->memSize = staticDataSize;
 
   if (!config->stackFirst)
     placeStack();
 
   if (WasmSym::heapBase) {
-    // Set `__heap_base` to directly follow the end of the stack or global data.
-    // The fact that this comes last means that a malloc/brk implementation
-    // can grow the heap at runtime.
+    // Set `__heap_base` to follow the end of the stack or global data. The
+    // fact that this comes last means that a malloc/brk implementation can
+    // grow the heap at runtime.
+    // We'll align the heap base here because memory allocators might expect
+    // __heap_base to be aligned already.
+    memoryPtr = alignTo(memoryPtr, heapAlignment);
     log("mem: heap base   = " + Twine(memoryPtr));
     WasmSym::heapBase->setVA(memoryPtr);
   }
 
   uint64_t maxMemorySetting = 1ULL
                               << (config->is64.getValueOr(false) ? 48 : 32);
 
   if (config->initialMemory != 0) {