[WebAssembly] Add missing implementation for --initial/max-memory args

[ncw]

Summary

Previously, Config->InitialMemory/MaxMemory were hooked up to some commandline args but had no effect at all.

[sbc100]

Code looks good. I have concerns about adding flags that don't have current users. Can you point to at least one before we add this?

[ncw]

In D44393#1035246, @sbc100 wrote:

Code looks good. I have concerns about adding flags that don't have current users. Can you point to at least one before we add this?

Good question - it's not so much adding a flag, as fixing an existing broken flag, but you're right. I'm not aware of any current users, but I was planning to use it myself in my own projects, to help catching any memory leaks.

[ruiu]

lld/trunk/wasm/Writer.cpp

611 ↗

(On Diff #138343)

This can be Config->InitMemory % WasmPageSize != 0

612 ↗

(On Diff #138343)

Something like

error: -initial-memory=<value>: value is not aligned to 4096 bytes

is more common style of an error message.

615 ↗

(On Diff #138343)

This else looks odd because if there is an error in the first if, the control reaches here, but if there is an error in the second if, it doesn't. Maybe we should just set it unconditionally. We won't use a value if there's an error because we'll exit before we use a dummy value.

[ncw]

lld/trunk/wasm/Writer.cpp

611 ↗

(On Diff #138343)

OK, it's just copied from the block above however where the same idiom is used. Should compile down to the same thing.

612 ↗

(On Diff #138343)

OK, I'll tidy that in a future commit. Thanks.

615 ↗

(On Diff #138343)

You say we'll exit before we use a dummy value - but I don't think exit is no-return?

In particular, if exit returns then we'll carry on to the MaxMemory block below, which uses MemoryPtr, so we have to leave this block with a "safe" value for MemoryPtr.

That was my logic anyway.

[ruiu]

lld/trunk/wasm/Writer.cpp

615 ↗

(On Diff #138343)

I mean error() counts the number of errors we've seen and before calling write(), we first check if the counter is zero, and if not, we return without calling that function (which will result in existing from the program.) So we do not really care too much about a wrong value in Config.

And with this code, a wrong value could still be set to MemoryPtr. If the first error test passes but the second test succeeds, this line is executed. That's inconsistent.

test/wasm/data-layout.ll

 ; RUN: llc -filetype=obj %p/Inputs/hello.ll -o %t.hello.o
 ; RUN: llc -filetype=obj %s -o %t.o
 
 target triple = "wasm32-unknown-unknown-wasm"
 
 @foo = hidden global i32 1, align 4
 @aligned_bar = hidden global i32 3, align 16
 
 @hello_str = external global i8*
 @external_ref = global i8** @hello_str, align 8
 
 ; RUN: wasm-ld -no-gc-sections --check-signatures --allow-undefined -o %t.wasm %t.o %t.hello.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
-; CHECK:        - Type:            GLOBAL
+; CHECK:        - Type:            MEMORY
+; CHECK-NEXT:     Memories:
+; CHECK-NEXT:       - Initial:         0x00000002
+; CHECK-NEXT:   - 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:           66608
 ; CHECK-NEXT:       - Index:           1

 ; CHECK-NEXT:       - SectionOffset:   41
 ; CHECK-NEXT:         MemoryIndex:     0
 ; CHECK-NEXT:         Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1052
 ; CHECK-NEXT:         Content:         68656C6C6F0A00
 
 
+; RUN: wasm-ld -no-gc-sections --check-signatures --allow-undefined \
+; RUN:     --initial-memory=131072 --max-memory=131072 -o %t_max.wasm %t.o \
+; RUN:     %t.hello.o
+; RUN: obj2yaml %t_max.wasm | FileCheck %s -check-prefix=CHECK-MAX
+
+; CHECK-MAX:        - Type:            MEMORY
+; CHECK-MAX-NEXT:     Memories:
+; CHECK-MAX-NEXT:       - Flags:           [ HAS_MAX ]
+; CHECK-MAX-NEXT:         Initial:         0x00000002
+; CHECK-MAX-NEXT:         Maximum:         0x00000002
+
+
 ; RUN: wasm-ld --check-signatures --relocatable -o %t_reloc.wasm %t.o %t.hello.o
 ; RUN: obj2yaml %t_reloc.wasm | FileCheck %s -check-prefix=RELOC
 
 ; RELOC:       - Type:            DATA
 ; RELOC-NEXT:     Relocations:
 ; RELOC-NEXT:       - Type:            R_WEBASSEMBLY_MEMORY_ADDR_I32
 ; RELOC-NEXT:         Index:           4
 ; RELOC-NEXT:         Offset:          0x00000018

test/wasm/import-memory.test

 # RUN: llc -filetype=obj %p/Inputs/start.ll -o %t.start.o
 # RUN: wasm-ld --check-signatures --import-memory -o %t.wasm %t.start.o
 # RUN: obj2yaml %t.wasm | FileCheck %s
 
 # Verify the --import-memory flag creates a memory import
 
 # CHECK:       - Type:            IMPORT
 # CHECK-NEXT:    Imports:
 # CHECK-NEXT:      - Module:          env
 # CHECK-NEXT:        Field:           memory
 # CHECK-NEXT:        Kind:            MEMORY
 # CHECK-NEXT:        Memory:
 # CHECK-NEXT:          Initial:         0x00000002
+# CHECK-NEXT:  - Type:
+
+
+
+# RUN: wasm-ld --check-signatures --import-memory --initial-memory=262144 \
+# RUN:     --max-memory=327680 -o %t.max.wasm %t.start.o
+# RUN: obj2yaml %t.max.wasm | FileCheck -check-prefix=CHECK-MAX %s
+
+# Verify the --initial-memory and --max-memory arguments work with imports
+
+# CHECK-MAX:       - Type:            IMPORT
+# CHECK-MAX-NEXT:    Imports:
+# CHECK-MAX-NEXT:      - Module:          env
+# CHECK-MAX-NEXT:        Field:           memory
+# CHECK-MAX-NEXT:        Kind:            MEMORY
+# CHECK-MAX-NEXT:        Memory:
+# CHECK-MAX-NEXT:          Flags:           [ HAS_MAX ]
+# CHECK-MAX-NEXT:          Initial:         0x00000004
+# CHECK-MAX-NEXT:          Maximum:         0x00000005
+# CHECK-MAX-NEXT:  - Type:

wasm/Writer.cpp

private:
   void createLinkingSection();
   void createNameSection();
 
   void writeHeader();
   void writeSections();
 
   uint64_t FileSize = 0;
   uint32_t NumMemoryPages = 0;
+  uint32_t MaxMemoryPages = 0;
 
   std::vector<const WasmSignature *> Types;
   DenseMap<WasmSignature, int32_t, WasmSignatureDenseMapInfo> TypeIndices;
   std::vector<const Symbol *> ImportedSymbols;
   unsigned NumImportedFunctions = 0;
   unsigned NumImportedGlobals = 0;
   std::vector<Symbol *> ExportedSymbols;
   std::vector<const DefinedData *> DefinedFakeGlobals;
void Writer::createImportSection() {
 
   if (Config->ImportMemory) {
     WasmImport Import;
     Import.Module = "env";
     Import.Field = "memory";
     Import.Kind = WASM_EXTERNAL_MEMORY;
     Import.Memory.Flags = 0;
     Import.Memory.Initial = NumMemoryPages;
+    if (MaxMemoryPages != 0) {
+      Import.Memory.Flags |= WASM_LIMITS_FLAG_HAS_MAX;
+      Import.Memory.Maximum = MaxMemoryPages;
+    }
     writeImport(OS, Import);
   }
 
   for (const Symbol *Sym : ImportedSymbols) {
     WasmImport Import;
     Import.Module = "env";
     Import.Field = Sym->getName();
     if (auto *FunctionSym = dyn_cast<FunctionSymbol>(Sym)) {

 
 void Writer::createMemorySection() {
   if (Config->ImportMemory)
     return;
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_MEMORY);
   raw_ostream &OS = Section->getStream();
 
+  bool HasMax = MaxMemoryPages != 0;
   writeUleb128(OS, 1, "memory count");
-  writeUleb128(OS, 0, "memory limits flags");
+  writeUleb128(OS, HasMax ? WASM_LIMITS_FLAG_HAS_MAX : 0, "memory limits flags");
   writeUleb128(OS, NumMemoryPages, "initial pages");
+  if (HasMax)
+    writeUleb128(OS, MaxMemoryPages, "max pages");
 }
 
 void Writer::createGlobalSection() {
   unsigned NumGlobals = InputGlobals.size() + DefinedFakeGlobals.size();
   if (NumGlobals == 0)
     return;
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_GLOBAL);
if (!Config->Relocatable) {
 
     // Set `__heap_base` to directly follow the end of the stack.  We don't
     // allocate any heap memory up front, but instead really on the malloc/brk
     // implementation growing the memory at runtime.
     WasmSym::HeapBase->setVirtualAddress(MemoryPtr);
     debugPrint("mem: heap base   = %d\n", MemoryPtr);
   }
 
+  if (Config->InitialMemory != 0) {
+    if (Config->InitialMemory != alignTo(Config->InitialMemory, WasmPageSize))
+      error("initial memory must be " + Twine(WasmPageSize) + "-byte aligned");
+    if (MemoryPtr > Config->InitialMemory)
+      error("initial memory too small, " + Twine(MemoryPtr) + " bytes needed");
+    else
+      MemoryPtr = Config->InitialMemory;
+  }
   uint32_t MemSize = alignTo(MemoryPtr, WasmPageSize);
   NumMemoryPages = MemSize / WasmPageSize;
   debugPrint("mem: total pages = %d\n", NumMemoryPages);
+
+  if (Config->MaxMemory != 0) {
+    if (Config->MaxMemory != alignTo(Config->MaxMemory, WasmPageSize))
+      error("maximum memory must be " + Twine(WasmPageSize) + "-byte aligned");
+    if (MemoryPtr > Config->MaxMemory)
+      error("maximum memory too small, " + Twine(MemoryPtr) + " bytes needed");
+    MaxMemoryPages = Config->MaxMemory / WasmPageSize;
+    debugPrint("mem: max pages   = %d\n", MaxMemoryPages);
+  }
 }
 
 SyntheticSection *Writer::createSyntheticSection(uint32_t Type,
                                                  StringRef Name) {
   auto Sec = make<SyntheticSection>(Type, Name);
   log("createSection: " + toString(*Sec));
   OutputSections.push_back(Sec);
   return Sec;