[WebAssembly] Fixed Writer::createInitMemoryFunction to work for wasm64

[aardappel]

Summary

Intended to fix: https://github.com/WebAssembly/memory64/issues/12

[aardappel]

Looks like we don't have a wasm32 test for this?

[tlively]

Hmm, I guess we don't. Would you mind adding one?

[sbc100]

I believe there is some coverage of createInitMemoryFunction in the tests.. although maybe not in a very useful/readable way.

Any test that checks for __wasm_init_memory is likely testing the output of this function:

$ git grep __wasm_init_memory lld/test/wasm/
lld/test/wasm/data-segment-merging.ll:; PASSIVE-MERGE-NEXT:        Name:            __wasm_init_memory
lld/test/wasm/data-segment-merging.ll:; PASSIVE-SEPARATE-NEXT:          Name:            __wasm_init_memory
lld/test/wasm/data-segments.ll:; PASSIVE-NEXT:        Name:            __wasm_init_memory
lld/test/wasm/tls.s:# Skip __wasm_call_ctors and __wasm_init_memory

Because the disassembly in objdump is not yet good enough to give good results on executables (it works OK for object files I think), its quite hard to write wasm-ld tests that verify the instructions themselves so we end up with tests that check the raw bytes of the function bodies.

For example in lld/test/wasm/data-segments.ll we have:

; PASSIVE-LABEL: - Type:            CODE                                         
; PASSIVE-NEXT:    Functions:                                                    
; PASSIVE-NEXT:      - Index:           0                                        
; PASSIVE-NEXT:        Locals:          []                                       
; PASSIVE-NEXT:        Body:            0B                                       
; PASSIVE-NEXT:      - Index:           1                                        
; PASSIVE-NEXT:        Locals:          []                                       
; PASSIVE-NEXT:        Body:            41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B

Here the second body corresponds to __wasm_init_memory I believe.

Admittedly is not great.. but I think it counts a test for now until we can get objdump -d working a bit better. (I have some ideas about next steps for that if you'd like to work on it).

[tlively]

Oh right, I forgot about those crazy unreadable tests. Sam is totally right. For now adding new RUN and CHECK lines to those tests should be sufficient.

[aardappel]

Well, these tests are not going to tell us if the generated instructions make any sense, but ok..

[sbc100]

In D92348#2431841, @aardappel wrote:

Well, these tests are not going to tell us if the generated instructions make any sense, but ok..

Indeed, that would requiring fixing objdump -d to work with executables.

I had a PR I was working on to make this work by creating dummy symbols for each function in the object file reader (in the case where no symbol table was found). That way every function (even in a stripped executable) would always appear to have some kind of symbol name to the disassembler.

[aardappel]

Has new tests that pass, and checked by hand that the one test where the code contents changed passes wasm-validate :)

[tlively]

lld/test/wasm/data-segment-merging.ll

150

If you replaced --check-prefix=PASSIVE-MERGE above with --check-prefixes=PASSIVE-MERGE,PASSIVE-MERGE32 and if you used --check-prefixes=PASSIVE-MERGE,PASSIVE-MERGE64 in this new run line, then you could avoid duplicating all of the context here by using PASSIVE-MERGE for everything that is shared and the *32 and *64 prefixes for only the lines that are different. Same for the PASSIVE-SEPARATE checks.

[aardappel]

Simplified tests

[tlively]

LGTM besides those last couple questions!

lld/wasm/Writer.cpp

927

Does this need to conditionally be a i64.atomic.wait?

954

Conditionally i64.atomic.store?

[aardappel]

lld/wasm/Writer.cpp

954

I believe the i32 refers to be value being stored, not the pointer size.

lld/test/wasm/data-segment-merging.ll

-target triple = "wasm32-unknown-unknown"
-
 @a = hidden global [6 x i8] c"hello\00", align 1
 @b = hidden global [8 x i8] c"goodbye\00", align 1
 @c = hidden global [9 x i8] c"whatever\00", align 1
 @d = hidden global i32 42, align 4
 
 @e = private constant [9 x i8] c"constant\00", align 1
 @f = private constant i8 43, align 4
 
-; RUN: llc -mattr=+bulk-memory,+atomics -filetype=obj %s -o %t.passive.o
-; RUN: llc -filetype=obj %s -o %t.o
+; RUN: llc --mtriple=wasm32-unknown-unknown -mattr=+bulk-memory,+atomics -filetype=obj %s -o %t.passive.o
+; RUN: llc --mtriple=wasm64-unknown-unknown -mattr=+bulk-memory,+atomics -filetype=obj %s -o %t.passive64.o
+; RUN: llc --mtriple=wasm32-unknown-unknown -filetype=obj %s -o %t.o
 
 ; RUN: wasm-ld -no-gc-sections --no-entry -o %t.merged.wasm %t.o
 ; RUN: obj2yaml %t.merged.wasm | FileCheck %s --check-prefix=MERGE
 
 ; MERGE-LABEL: - Type:            DATA
 ; MERGE-NEXT:    Segments:
 ; MERGE-NEXT:      - SectionOffset:   7
 ; MERGE-NEXT:        InitFlags:       0

 ; SEPARATE-NEXT:        Name:            __wasm_call_ctors
 ; SEPARATE-NEXT:    GlobalNames:
 ; SEPARATE-NEXT:      - Index:           0
 ; SEPARATE-NEXT:        Name:            __stack_pointer
 ; SEPARATE-NOT:       - Index:
 
 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 -o %t.merged.passive.wasm %t.passive.o
 ; RUN: obj2yaml %t.merged.passive.wasm | FileCheck %s --check-prefix=PASSIVE-MERGE
+; RUN: wasm-ld -mwasm64 -no-gc-sections --no-entry --shared-memory --max-memory=131072 -o %t.merged.passive64.wasm %t.passive64.o
+; RUN: obj2yaml %t.merged.passive64.wasm | FileCheck %s --check-prefix=PASSIVE-MERGE
 
 ; PASSIVE-MERGE-LABEL: - Type:            DATACOUNT
 ; PASSIVE-MERGE-NEXT:    Count:           2
 ; PASSIVE-MERGE-LABEL: - Type:            DATA
 ; PASSIVE-MERGE-NEXT:    Segments:
 ; PASSIVE-MERGE-NEXT:      - SectionOffset:   3
 ; PASSIVE-MERGE-NEXT:        InitFlags:       1
 ; PASSIVE-MERGE-NEXT:        Content:         636F6E7374616E74000000002B
 ; PASSIVE-MERGE-NEXT:      - SectionOffset:   18
 ; PASSIVE-MERGE-NEXT:        InitFlags:       1
 ; PASSIVE-MERGE-NEXT:        Content:         68656C6C6F00676F6F6462796500776861746576657200002A000000
 ; PASSIVE-MERGE-NEXT:  - Type:            CUSTOM
 ; PASSIVE-MERGE-NEXT:    Name:            name
 ; PASSIVE-MERGE-NEXT:    FunctionNames:
 ; PASSIVE-MERGE-NEXT:      - Index:           0
 ; PASSIVE-MERGE-NEXT:        Name:            __wasm_call_ctors
 ; PASSIVE-MERGE-NEXT:      - Index:           1
 ; PASSIVE-MERGE-NEXT:        Name:            __wasm_init_memory
 ; PASSIVE-MERGE-NEXT:      - Index:           2
 ; PASSIVE-MERGE-NEXT:        Name:            __wasm_init_tls
 
 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 -no-merge-data-segments -o %t.separate.passive.wasm %t.passive.o
 ; RUN: obj2yaml %t.separate.passive.wasm | FileCheck %s --check-prefix=PASSIVE-SEPARATE
+; RUN: wasm-ld -mwasm64 -no-gc-sections --no-entry --shared-memory --max-memory=131072 -no-merge-data-segments -o %t.separate.passive64.wasm %t.passive64.o
+; RUN: obj2yaml %t.separate.passive64.wasm | FileCheck %s --check-prefix=PASSIVE-SEPARATE
 
 ; PASSIVE-SEPARATE-LABEL: - Type:            DATACOUNT
 ; PASSIVE-SEPARATE-NEXT:    Count:           6
 ; PASSIVE-SEPARATE-LABEL: - Type:            DATA
 ; PASSIVE-SEPARATE-NEXT:    Segments:
 ; PASSIVE-SEPARATE-NEXT:      - SectionOffset:   3
 ; PASSIVE-SEPARATE-NEXT:        InitFlags:       1
 ; PASSIVE-SEPARATE-NEXT:        Content:         636F6E7374616E7400

 ; PASSIVE-SEPARATE-NEXT:    - Type:            CUSTOM
 ; PASSIVE-SEPARATE-NEXT:      Name:            name
 ; PASSIVE-SEPARATE-NEXT:      FunctionNames:
 ; PASSIVE-SEPARATE-NEXT:        - Index:           0
 ; PASSIVE-SEPARATE-NEXT:          Name:            __wasm_call_ctors
 ; PASSIVE-SEPARATE-NEXT:        - Index:           1
 ; PASSIVE-SEPARATE-NEXT:          Name:            __wasm_init_memory
 ; PASSIVE-SEPARATE-NEXT:        - Index:           2
 ; PASSIVE-SEPARATE-NEXT:          Name:            __wasm_init_tls

lld/test/wasm/data-segments.ll

-; RUN: llc -filetype=obj %s -o %t.atomics.o -mattr=+atomics
-; RUN: llc -filetype=obj %s -o %t.bulk-mem.o -mattr=+bulk-memory
-; RUN: llc -filetype=obj %s -o %t.atomics.bulk-mem.o -mattr=+atomics,+bulk-memory
+; RUN: llc --mtriple=wasm32-unknown-unknown -filetype=obj %s -o %t.atomics.o -mattr=+atomics
+; RUN: llc --mtriple=wasm32-unknown-unknown -filetype=obj %s -o %t.bulk-mem.o -mattr=+bulk-memory
+; RUN: llc --mtriple=wasm32-unknown-unknown -filetype=obj %s -o %t.atomics.bulk-mem.o -mattr=+atomics,+bulk-memory
+; RUN: llc --mtriple=wasm64-unknown-unknown -filetype=obj %s -o %t.atomics.bulk-mem64.o -mattr=+atomics,+bulk-memory
 
 ; atomics, shared memory => error
 ; RUN: not wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.o -o %t.atomics.wasm 2>&1 | FileCheck %s --check-prefix ERROR
 
 ; bulk memory, unshared memory => active segments
 ; RUN: wasm-ld -no-gc-sections --no-entry %t.bulk-mem.o -o %t.bulk-mem.wasm
 ; RUN: obj2yaml %t.bulk-mem.wasm | FileCheck %s --check-prefix ACTIVE
 
 ; atomics, bulk memory, shared memory => passive segments
 ; RUN: wasm-ld -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem.o -o %t.atomics.bulk-mem.wasm
-; RUN: obj2yaml %t.atomics.bulk-mem.wasm | FileCheck %s --check-prefixes PASSIVE
+; RUN: obj2yaml %t.atomics.bulk-mem.wasm | FileCheck %s --check-prefixes PASSIVE,PASSIVE32
+
+; Also test with wasm64
+; RUN: wasm-ld -mwasm64 -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem64.o -o %t.atomics.bulk-mem64.wasm
+; RUN: obj2yaml %t.atomics.bulk-mem64.wasm | FileCheck %s --check-prefixes PASSIVE,PASSIVE64
 
 ; Also test in combination with PIC/pie
-; RUN: llc -filetype=obj -relocation-model=pic %s -o %t.atomics.bulk-mem.pic.o -mattr=+atomics,+bulk-memory,+mutable-globals
+; RUN: llc --mtriple=wasm32-unknown-unknown -filetype=obj -relocation-model=pic %s -o %t.atomics.bulk-mem.pic.o -mattr=+atomics,+bulk-memory,+mutable-globals
 ; RUN: wasm-ld --experimental-pic -pie -no-gc-sections --no-entry --shared-memory --max-memory=131072 %t.atomics.bulk-mem.pic.o -o %t.pic.wasm
 
-target triple = "wasm32-unknown-unknown"
-
 @a = hidden global [6 x i8] c"hello\00", align 1
 @b = hidden global [8 x i8] c"goodbye\00", align 1
 @c = hidden global [10000 x i8] zeroinitializer, align 1
 @d = hidden global i32 42, align 4
 
 @e = private constant [9 x i8] c"constant\00", align 1
 @f = private constant i8 43, align 4
 

 ; PASSIVE-NEXT:    Count:           2
 ; PASSIVE-LABEL: - Type:            CODE
 ; PASSIVE-NEXT:    Functions:
 ; PASSIVE-NEXT:      - Index:           0
 ; PASSIVE-NEXT:        Locals:          []
 ; PASSIVE-NEXT:        Body:            0B
 ; PASSIVE-NEXT:      - Index:           1
 ; PASSIVE-NEXT:        Locals:          []
-; PASSIVE-NEXT:        Body:            41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B
+; PASSIVE32-NEXT:        Body:            41B4D60041004101FE480200044041B4D6004101427FFE0102001A054180084100410DFC08000041900841004114FC08010041B4D6004102FE17020041B4D600417FFE0002001A0BFC0900FC09010B
+; PASSIVE64-NEXT:        Body:            42B4D60041004101FE480200044042B4D6004101427FFE0102001A054280084100410DFC08000042900841004114FC08010042B4D6004102FE17020042B4D600417FFE0002001A0BFC0900FC09010B
 
 ; PASSIVE-NEXT:  - Index:           2
 ; PASSIVE-NEXT:    Locals:          []
 ; PASSIVE-NEXT:    Body:            0B
 ; PASSIVE-NEXT:  - Type:            DATA
 ; PASSIVE-NEXT:    Segments:
 ; PASSIVE-NEXT:      - SectionOffset:   3
 ; PASSIVE-NEXT:        InitFlags:       1

lld/wasm/Writer.cpp

return std::find_if(segments.begin(), segments.end(),
                       [this](const OutputSegment *s) {
                         return this->needsPassiveInitialization(s);
                       }) != segments.end();
 }
 
 void Writer::createInitMemoryFunction() {
   LLVM_DEBUG(dbgs() << "createInitMemoryFunction\n");
   assert(WasmSym::initMemoryFlag);
-  uint32_t flagAddress = WasmSym::initMemoryFlag->getVirtualAddress();
+  uint64_t flagAddress = WasmSym::initMemoryFlag->getVirtualAddress();
   std::string bodyContent;
   {
     raw_string_ostream os(bodyContent);
     writeUleb128(os, 0, "num locals");
 
     if (hasPassiveInitializedSegments()) {
       // Initialize memory in a thread-safe manner. The thread that successfully
       // increments the flag from 0 to 1 is is responsible for performing the
if (hasPassiveInitializedSegments()) {
       //      (i32.const -1u)
       //     )
       //    )
       //   )
       //  )
       //  ( ... drop data segments ... )
       // )
 
+      bool is64 = config->is64.getValueOr(false);
+
       // Atomically check whether this is the main thread.
-      writeI32Const(os, flagAddress, "flag address");
+      writePtrConst(os, flagAddress, is64, "flag address");
       writeI32Const(os, 0, "expected flag value");
       writeI32Const(os, 1, "flag value");
       writeU8(os, WASM_OPCODE_ATOMICS_PREFIX, "atomics prefix");
       writeUleb128(os, WASM_OPCODE_I32_RMW_CMPXCHG, "i32.atomic.rmw.cmpxchg");
       writeMemArg(os, 2, 0);
       writeU8(os, WASM_OPCODE_IF, "IF");
       writeU8(os, WASM_TYPE_NORESULT, "blocktype");
 
       // Did not increment 0, so wait for main thread to initialize memory
-      writeI32Const(os, flagAddress, "flag address");
+      writePtrConst(os, flagAddress, is64, "flag address");
       writeI32Const(os, 1, "expected flag value");
       writeI64Const(os, -1, "timeout");
+
       writeU8(os, WASM_OPCODE_ATOMICS_PREFIX, "atomics prefix");
       writeUleb128(os, WASM_OPCODE_I32_ATOMIC_WAIT, "i32.atomic.wait");
       writeMemArg(os, 2, 0);
       writeU8(os, WASM_OPCODE_DROP, "drop");
 
       writeU8(os, WASM_OPCODE_ELSE, "ELSE");
 
       // Did increment 0, so conditionally initialize passive data segments
       for (const OutputSegment *s : segments) {
         if (needsPassiveInitialization(s)) {
           // destination address
-          if (config->is64.getValueOr(false)) {
-            writeI64Const(os, s->startVA, "destination address");
-          } else {
-            writeI32Const(os, static_cast<int32_t>(s->startVA),
-                          "destination address");
-          }
+          writePtrConst(os, s->startVA, is64, "destination address");
           // source segment offset
           writeI32Const(os, 0, "segment offset");
           // memory region size
           writeI32Const(os, s->size, "memory region size");
           // memory.init instruction
           writeU8(os, WASM_OPCODE_MISC_PREFIX, "bulk-memory prefix");
           writeUleb128(os, WASM_OPCODE_MEMORY_INIT, "memory.init");
           writeUleb128(os, s->index, "segment index immediate");
           writeU8(os, 0, "memory index immediate");
         }
       }
 
       // Set flag to 2 to mark end of initialization
-      writeI32Const(os, flagAddress, "flag address");
+      writePtrConst(os, flagAddress, is64, "flag address");
       writeI32Const(os, 2, "flag value");
       writeU8(os, WASM_OPCODE_ATOMICS_PREFIX, "atomics prefix");
       writeUleb128(os, WASM_OPCODE_I32_ATOMIC_STORE, "i32.atomic.store");
       writeMemArg(os, 2, 0);
 
       // Notify any waiters that memory initialization is complete
-      writeI32Const(os, flagAddress, "flag address");
+      writePtrConst(os, flagAddress, is64, "flag address");
       writeI32Const(os, -1, "number of waiters");
       writeU8(os, WASM_OPCODE_ATOMICS_PREFIX, "atomics prefix");
       writeUleb128(os, WASM_OPCODE_ATOMIC_NOTIFY, "atomic.notify");
       writeMemArg(os, 2, 0);
       writeU8(os, WASM_OPCODE_DROP, "drop");
 
       writeU8(os, WASM_OPCODE_END, "END");
 

lld/wasm/WriterUtils.h

void writeValueType(raw_ostream &os, llvm::wasm::ValType type,
                     const Twine &msg);
 
 void writeSig(raw_ostream &os, const llvm::wasm::WasmSignature &sig);
 
 void writeI32Const(raw_ostream &os, int32_t number, const Twine &msg);
 
 void writeI64Const(raw_ostream &os, int64_t number, const Twine &msg);
 
+void writePtrConst(raw_ostream &os, int64_t number, bool is64,
+                   const Twine &msg);
+
 void writeMemArg(raw_ostream &os, uint32_t alignment, uint64_t offset);
 
 void writeInitExpr(raw_ostream &os, const llvm::wasm::WasmInitExpr &initExpr);
 
 void writeLimits(raw_ostream &os, const llvm::wasm::WasmLimits &limits);
 
 void writeGlobalType(raw_ostream &os, const llvm::wasm::WasmGlobalType &type);
 

lld/wasm/WriterUtils.cpp

void writeI32Const(raw_ostream &os, int32_t number, const Twine &msg) {
   writeSleb128(os, number, msg);
 }
 
 void writeI64Const(raw_ostream &os, int64_t number, const Twine &msg) {
   writeU8(os, WASM_OPCODE_I64_CONST, "i64.const");
   writeSleb128(os, number, msg);
 }
 
+void writePtrConst(raw_ostream &os, int64_t number, bool is64,
+                   const Twine &msg) {
+  if (is64)
+    writeI64Const(os, number, msg);
+  else
+    writeI32Const(os, static_cast<int32_t>(number), msg);
+}
+
 void writeMemArg(raw_ostream &os, uint32_t alignment, uint64_t offset) {
   writeUleb128(os, alignment, "alignment");
   writeUleb128(os, offset, "offset");
 }
 
 void writeInitExpr(raw_ostream &os, const WasmInitExpr &initExpr) {
   writeU8(os, initExpr.Opcode, "opcode");
   switch (initExpr.Opcode) {