[wasm-ld] Allow importing/exporting the output module's memory with arbitrary names

[Liamolucko]

Summary

Adds an --export-memory option to wasm-ld which allows passing a name to give to the exported memory, and extends --import-memory to allow passing a <module>,<name> pair specifying where the memory should be imported from.

This is to support the newly-defined 'Canonical ABI' for modules (https://github.com/WebAssembly/component-model/pull/71), which requires that the memory be exported as "cabi_memory".

[sbc100]

I think we maybe want to think this over a little more. Clearly there is a need to be able to control the import/export of memory 0 but I'd like to make sure we do it in a forward thinking way.

Some considerations.

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?
2. When we went to multi-table we made table 0 into a first class symbol.. then it could be reasonable like any other symbol. Perhaps we should do this for memory 0 too? See https://reviews.llvm.org/D96001
3. It would be to be able to control the default "env" name from which all default import come. This could remove the needs for the comma separated pair parsing.

If we made __linear_memory into first class symbol like __indirection_function_table then perhaps we could allow the toolchain to control its import/export name directly using the import_name/export_name attributes in assembly and avoid the need for any new command line flag and re-using the existing machinery?

[sbc100]

lld/wasm/Driver.cpp

379

Why allow the same memory to be exported multiple times under different names?

I guess it could be useful but I'd like to see it done in a more standard way that could be re-used for all symbols types (and looking forward to multiple memories).

We have support for symbols alias in the assembly format and object format so I would hope would achieve multiple names that way if possible.

[Liamolucko]

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?

Yeah, the linker will also have to merge together cabi_start definitions, so using a unified flag makes more sense. I'll make a new patch to do it that way. Does Phabricator have a way to close this one?

If we made __linear_memory into first class symbol like __indirection_function_table then perhaps we could allow the toolchain to control its import/export name directly using the import_name/export_name attributes in assembly and avoid the need for any new command line flag and re-using the existing machinery?

Yeah, that seems like the proper way to allow renaming. That would require adding a new SYMTAB_MEMORY to https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md#linking-metadata-section, right?

[sbc100]

In D131376#3711300, @Liamolucko wrote:

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?

Yeah, the linker will also have to merge together cabi_start definitions, so using a unified flag makes more sense. I'll make a new patch to do it that way. Does Phabricator have a way to close this one?

If we made __linear_memory into first class symbol like __indirection_function_table then perhaps we could allow the toolchain to control its import/export name directly using the import_name/export_name attributes in assembly and avoid the need for any new command line flag and re-using the existing machinery?

Yeah, that seems like the proper way to allow renaming. That would require adding a new SYMTAB_MEMORY to https://github.com/WebAssembly/tool-conventions/blob/main/Linking.md#linking-metadata-section, right?

Yes, I think so, plus a bunch of work in MC and wasm-ld layers to support multiple memories. This work was already done for the multi-table use case, so we should be able to follow that pattern.

[sbc100]

In D131376#3711300, @Liamolucko wrote:

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?

Yeah, the linker will also have to merge together cabi_start definitions, so using a unified flag makes more sense. I'll make a new patch to do it that way. Does Phabricator have a way to close this one?

That does cabi_start represent? If its the equivalent of main we probably don't want the linker to merge them but instead reject programs that contain more than one (just like any other symbol).

[Liamolucko]

In D131376#3716714, @sbc100 wrote:

In D131376#3711300, @Liamolucko wrote:

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?

Yeah, the linker will also have to merge together cabi_start definitions, so using a unified flag makes more sense. I'll make a new patch to do it that way. Does Phabricator have a way to close this one?

That does cabi_start represent? If its the equivalent of main we probably don't want the linker to merge them but instead reject programs that contain more than one (just like any other symbol).

It's the component-model equivalent of WebAssembly start functions, which can be used for main but are mainly meant for initialization. Fully-fledged components are explicitly allowed to have multiple, so I think it makes sense to merge them.

[sunfish]

In curious why memorytExports is a set of strings rather than an optional string. Is there a need for exporting the memory with multiple names?

I can envision a possible need, which is to build modules that export both "memory" and "cabi_memory". I'm open to supporting something like that if there's an acute need for it, but I'm also hesitant, because

In D131376#3720931, @Liamolucko wrote:

In D131376#3716714, @sbc100 wrote:

In D131376#3711300, @Liamolucko wrote:

1. Are there going to be other cabi-relatated things, that warrent just putting this behind a single "cabi" flag rather than requiring N different flags for this?

Yeah, the linker will also have to merge together cabi_start definitions, so using a unified flag makes more sense. I'll make a new patch to do it that way. Does Phabricator have a way to close this one?

That does cabi_start represent? If its the equivalent of main we probably don't want the linker to merge them but instead reject programs that contain more than one (just like any other symbol).

It's the component-model equivalent of WebAssembly start functions, which can be used for main but are mainly meant for initialization. Fully-fledged components are explicitly allowed to have multiple, so I think it makes sense to merge them.

I think the canonical ABI will need support for multiple cabi_start functions, so that we don't have to merge them. I filled https://github.com/WebAssembly/component-model/issues/88 to track this.

[Liamolucko]

In D131376#3723463, @sunfish wrote:

In curious why memorytExports is a set of strings rather than an optional string. Is there a need for exporting the memory with multiple names?

Not really; I just thought that I would allow for arbitrary combinations while I was at it.

In D131376#3723463, @sunfish wrote:

I think the canonical ABI will need support for multiple cabi_start functions, so that we don't have to merge them. I filled https://github.com/WebAssembly/component-model/issues/88 to track this.

Now that it seems like that's not going to be supported natively in the Canonical ABI, should the linker merge together cabi_start functions? They shouldn't be able to depend on one another, but it seems like it could be useful to define cabi_start functions across different files that work with independent values.

[sbc100]

In D131376#3736824, @Liamolucko wrote:

In D131376#3723463, @sunfish wrote:

In curious why memorytExports is a set of strings rather than an optional string. Is there a need for exporting the memory with multiple names?

Not really; I just thought that I would allow for arbitrary combinations while I was at it.

In D131376#3723463, @sunfish wrote:

I think the canonical ABI will need support for multiple cabi_start functions, so that we don't have to merge them. I filled https://github.com/WebAssembly/component-model/issues/88 to track this.

Now that it seems like that's not going to be supported natively in the Canonical ABI, should the linker merge together cabi_start functions? They shouldn't be able to depend on one another, but it seems like it could be useful to define cabi_start functions across different files that work with independent values.

I think its probably simpler to only allow a single start cabi_start definition.. just like any other symbol. It sounds like it should be a linker error to have more than one of them.

[sunfish]

In D131376#3737184, @sbc100 wrote:

I think its probably simpler to only allow a single start cabi_start definition.. just like any other symbol. It sounds like it should be a linker error to have more than one of them.

This sounds right to me. We already have the .init_array mechanism for registering constructors at link time, and it sounds like we can continue to use that with cabi_start.

[sbc100]

Should this be closed in favor of https://reviews.llvm.org/D135898?

lld/test/wasm/memory-naming.test

+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %t.start.o
+# RUN: wasm-ld --export-memory=foo -o %t.wasm %t.start.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+
+# Verify that the --export-memory=<name> option changes the exported name of the module's memory
+
+# CHECK:       - Type:            EXPORT
+# CHECK-NEXT:    Exports:
+# CHECK-NEXT:      - Name:            foo
+# CHECK-NEXT:        Kind:            MEMORY
+# CHECK-NEXT:        Index:           0
+# CHECK-NEXT:      - Name:            _start
+# CHECK-NEXT:        Kind:            FUNCTION
+# CHECK-NEXT:        Index:           0
+# CHECK-NEXT:  - Type:
+
+# RUN:wasm-ld --export-memory --export-memory=foo -o %t.unnamed.wasm %t.start.o
+# RUN: obj2yaml %t.unnamed.wasm | FileCheck -check-prefix=CHECK-UNNAMED %s
+
+# Verify that the --export-memory option without a parameter exports the memory
+# as "memory"
+
+# CHECK-UNNAMED:       - Type:            EXPORT
+# CHECK-UNNAMED-NEXT:    Exports:
+# CHECK-UNNAMED-NEXT:      - Name:            foo
+# CHECK-UNNAMED-NEXT:        Kind:            MEMORY
+# CHECK-UNNAMED-NEXT:        Index:           0
+# CHECK-UNNAMED-NEXT:      - Name:            memory
+# CHECK-UNNAMED-NEXT:        Kind:            MEMORY
+# CHECK-UNNAMED-NEXT:        Index:           0
+# CHECK-UNNAMED-NEXT:      - Name:            _start
+# CHECK-UNNAMED-NEXT:        Kind:            FUNCTION
+# CHECK-UNNAMED-NEXT:        Index:           0
+# CHECK-UNNAMED-NEXT:  - Type:
+
+# RUN:wasm-ld --export-memory=foo --export-memory=foo -o %t.duplicate.wasm %t.start.o
+# RUN: obj2yaml %t.duplicate.wasm | FileCheck -check-prefix=CHECK-DUPLICATE %s
+
+# Verify that passing --export-memory with the same name twice works
+
+# CHECK-DUPLICATE:       - Type:            EXPORT
+# CHECK-DUPLICATE-NEXT:    Exports:
+# CHECK-DUPLICATE-NEXT:      - Name:            foo
+# CHECK-DUPLICATE-NEXT:        Kind:            MEMORY
+# CHECK-DUPLICATE-NEXT:        Index:           0
+# CHECK-DUPLICATE-NEXT:      - Name:            _start
+# CHECK-DUPLICATE-NEXT:        Kind:            FUNCTION
+# CHECK-DUPLICATE-NEXT:        Index:           0
+# CHECK-DUPLICATE-NEXT:  - Type:
+
+# RUN:wasm-ld --import-memory=foo,bar -o %t.import.wasm %t.start.o
+# RUN: obj2yaml %t.import.wasm | FileCheck -check-prefix=CHECK-IMPORT %s
+
+# Verify that memory imports can be renamed, and that no memory is exported by
+# default when memory is being imported
+
+# CHECK-IMPORT:       - Type:            IMPORT
+# CHECK-IMPORT-NEXT:    Imports:
+# CHECK-IMPORT-NEXT:      - Module:          foo
+# CHECK-IMPORT-NEXT:        Field:           bar
+# CHECK-IMPORT-NEXT:        Kind:            MEMORY
+# CHECK-IMPORT-NEXT:        Memory:
+# CHECK-IMPORT-NEXT:          Minimum:         0x2
+# CHECK-IMPORT:      - Type:            EXPORT
+# CHECK-IMPORT-NEXT:    Exports:
+# CHECK-IMPORT-NEXT:      - Name:            _start
+# CHECK-IMPORT-NEXT:        Kind:            FUNCTION
+# CHECK-IMPORT-NEXT:        Index:           0
+# CHECK-IMPORT-NEXT:  - Type:
+
+# RUN:wasm-ld --import-memory=foo,bar --export-memory --export-memory=qux -o %t.both.wasm %t.start.o
+# RUN: obj2yaml %t.both.wasm | FileCheck -check-prefix=CHECK-BOTH %s
+
+# Verify that memory can be both imported and exported from a module
+
+# CHECK-BOTH:       - Type:            IMPORT
+# CHECK-BOTH-NEXT:    Imports:
+# CHECK-BOTH-NEXT:      - Module:          foo
+# CHECK-BOTH-NEXT:        Field:           bar
+# CHECK-BOTH-NEXT:        Kind:            MEMORY
+# CHECK-BOTH-NEXT:        Memory:
+# CHECK-BOTH-NEXT:          Minimum:         0x2
+# CHECK-BOTH:       - Type:            EXPORT
+# CHECK-BOTH-NEXT:    Exports:
+# CHECK-BOTH-NEXT:      - Name:            memory
+# CHECK-BOTH-NEXT:        Kind:            MEMORY
+# CHECK-BOTH-NEXT:        Index:           0
+# CHECK-BOTH-NEXT:      - Name:            qux
+# CHECK-BOTH-NEXT:        Kind:            MEMORY
+# CHECK-BOTH-NEXT:        Index:           0
+# CHECK-BOTH-NEXT:      - Name:            _start
+# CHECK-BOTH-NEXT:        Kind:            FUNCTION
+# CHECK-BOTH-NEXT:        Index:           0
+# CHECK-BOTH-NEXT:  - Type:

lld/wasm/Config.h

struct Configuration {
   bool experimentalPic;
   bool emitRelocs;
   bool exportAll;
   bool exportDynamic;
   bool exportTable;
   bool extendedConst;
   bool growableTable;
   bool gcSections;
-  bool importMemory;
+  llvm::Optional<std::pair<llvm::StringRef, llvm::StringRef>> memoryImport;
+  llvm::StringSet<> memoryExports;
   bool sharedMemory;
   bool importTable;
   bool importUndefined;
   llvm::Optional<bool> is64;
   bool mergeDataSegments;
   bool pie;
   bool printGcSections;
   bool relocatable;

lld/wasm/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->demangle = args.hasFlag(OPT_demangle, OPT_no_demangle, true);
   config->disableVerify = args.hasArg(OPT_disable_verify);
   config->emitRelocs = args.hasArg(OPT_emit_relocs);
   config->experimentalPic = args.hasArg(OPT_experimental_pic);
   config->entry = getEntry(args);
   config->exportAll = args.hasArg(OPT_export_all);
   config->exportTable = args.hasArg(OPT_export_table);
   config->growableTable = args.hasArg(OPT_growable_table);
-  config->importMemory = args.hasArg(OPT_import_memory);
+
+  if (args.hasArg(OPT_import_memory_with_name)) {
+    config->memoryImport =
+        args.getLastArgValue(OPT_import_memory_with_name).split(",");
+  } else if (args.hasArg(OPT_import_memory)) {
+    config->memoryImport =
+        std::pair<llvm::StringRef, llvm::StringRef>("env", "memory");
+  } else {
+    config->memoryImport =
+        llvm::Optional<std::pair<llvm::StringRef, llvm::StringRef>>();
+  }
+
+  for (llvm::StringRef name :
+       args.getAllArgValues(OPT_export_memory_with_name)) {
+    config->memoryExports.insert(name);
+  }
+
+  if (args.hasArg(OPT_export_memory)) {
+    config->memoryExports.insert("memory");
+  }
+
   config->sharedMemory = args.hasArg(OPT_shared_memory);
   config->importTable = args.hasArg(OPT_import_table);
   config->importUndefined = args.hasArg(OPT_import_undefined);
   config->ltoo = args::getInteger(args, OPT_lto_O, 2);
   config->ltoPartitions = args::getInteger(args, OPT_lto_partitions, 1);
   config->ltoDebugPassManager = args.hasArg(OPT_lto_debug_pass_manager);
   config->mapFile = args.getLastArgValue(OPT_Map);
   config->optimize = args::getInteger(args, OPT_O, 1);
if (config->exportTable)
       error("--relocatable is incompatible with --export-table");
     if (config->growableTable)
       error("--relocatable is incompatible with --growable-table");
     // Ignore any --import-table, as it's redundant.
     config->importTable = true;
   }
 
   if (config->shared) {
-    config->importMemory = true;
+    if (!config->memoryImport.hasValue()) {
+      config->memoryImport =
+          std::pair<llvm::StringRef, llvm::StringRef>("env", "memory");
+    }
     config->importUndefined = true;
   }
 }
 
 // Some command line options or some combinations of them are not allowed.
 // This function checks for such errors.
 static void checkOptions(opt::InputArgList &args) {
   if (!config->stripDebug && !config->stripAll && config->compressRelocations)

lld/wasm/Options.td

 
 def growable_table: FF<"growable-table">,
   HelpText<"Remove maximum size from function table, allowing table to grow">;
 
 def global_base: JJ<"global-base=">,
   HelpText<"Where to start to place global data">;
 
 def import_memory: FF<"import-memory">,
-  HelpText<"Import memory from the environment">;
+  HelpText<"Import the module's memory from the default module of \"env\" with the name \"memory\".">;
+def import_memory_with_name: JJ<"import-memory=">,
+  HelpText<"Import the module's memory from the passed module with the passed name.">,
+  MetaVarName<"<module>,<name>">;
+
+def export_memory: FF<"export-memory">,
+  HelpText<"Export the module's memory with the default name of \"memory\"">;
+def export_memory_with_name: JJ<"export-memory=">,
+  HelpText<"Export the module's memory with the passed name">;
 
 def shared_memory: FF<"shared-memory">,
   HelpText<"Use shared linear memory">;
 
 def import_table: FF<"import-table">,
   HelpText<"Import function table from the environment">;
 
 def initial_memory: JJ<"initial-memory=">,

lld/wasm/SyntheticSections.h

public:
 
   std::vector<InputTable *> inputTables;
 };
 
 class MemorySection : public SyntheticSection {
 public:
   MemorySection() : SyntheticSection(llvm::wasm::WASM_SEC_MEMORY) {}
 
-  bool isNeeded() const override { return !config->importMemory; }
+  bool isNeeded() const override { return !config->memoryImport.hasValue(); }
   void writeBody() override;
 
   uint64_t numMemoryPages = 0;
   uint64_t maxMemoryPages = 0;
 };
 
 // The tag section contains a list of declared wasm tags associated with the
 // module. Currently the only supported tag kind is exceptions. All C++

lld/wasm/SyntheticSections.cpp

void TypeSection::writeBody() {
   writeUleb128(bodyOutputStream, types.size(), "type count");
   for (const WasmSignature *sig : types)
     writeSig(bodyOutputStream, *sig);
 }
 
 uint32_t ImportSection::getNumImports() const {
   assert(isSealed);
   uint32_t numImports = importedSymbols.size() + gotSymbols.size();
-  if (config->importMemory)
+  if (config->memoryImport.hasValue())
     ++numImports;
   return numImports;
 }
 
 void ImportSection::addGOTEntry(Symbol *sym) {
   assert(!isSealed);
   if (sym->hasGOTIndex())
     return;

 
 void ImportSection::writeBody() {
   raw_ostream &os = bodyOutputStream;
 
   writeUleb128(os, getNumImports(), "import count");
 
   bool is64 = config->is64.value_or(false);
 
-  if (config->importMemory) {
+  if (config->memoryImport.hasValue()) {
     WasmImport import;
-    import.Module = defaultModule;
-    import.Field = "memory";
+    import.Module = config->memoryImport.getValue().first;
+    import.Field = config->memoryImport.getValue().second;
     import.Kind = WASM_EXTERNAL_MEMORY;
     import.Memory.Flags = 0;
     import.Memory.Minimum = out.memorySec->numMemoryPages;
     if (out.memorySec->maxMemoryPages != 0 || config->sharedMemory) {
       import.Memory.Flags |= WASM_LIMITS_FLAG_HAS_MAX;
       import.Memory.Maximum = out.memorySec->maxMemoryPages;
     }
     if (config->sharedMemory)

lld/wasm/Writer.cpp

for (ObjFile *file : symtab->objectFiles) {
     }
   }
 
 done:
   // Normally we don't include bss segments in the binary.  In particular if
   // memory is not being imported then we can assume its zero initialized.
   // In the case the memory is imported, and we can use the memory.fill
   // instruction, then we can also avoid including the segments.
-  if (config->importMemory && !allowed.count("bulk-memory"))
+  if (config->memoryImport.hasValue() && !allowed.count("bulk-memory"))
     config->emitBssSegments = true;
 
   if (allowed.count("extended-const"))
     config->extendedConst = true;
 
   for (auto &feature : allowed)
     log("Allowed feature: " + feature);
 }
for (Symbol *sym : symtab->getSymbols()) {
     out.importSec->addImport(sym);
   }
 }
 
 void Writer::calculateExports() {
   if (config->relocatable)
     return;
 
-  if (!config->relocatable && !config->importMemory)
+  // If the module's memory isn't imported or exported under some other name,
+  // default to exporting it as `"memory"`.
+  if (!config->memoryImport.hasValue() && config->memoryExports.empty()) {
+    config->memoryExports.insert("memory");
+  }
+
+  for (StringMapEntry<NoneType> &entry : config->memoryExports) {
     out.exportSec->exports.push_back(
-        WasmExport{"memory", WASM_EXTERNAL_MEMORY, 0});
+        WasmExport{entry.getKey(), WASM_EXTERNAL_MEMORY, 0});
+  }
 
   unsigned globalIndex =
       out.importSec->getNumImportedGlobals() + out.globalSec->numGlobals();
 
   for (Symbol *sym : symtab->getSymbols()) {
     if (!sym->isExported())
       continue;
     if (!sym->isLive())
static void createFunction(DefinedFunction *func, StringRef bodyContent) {
   }
   ArrayRef<uint8_t> body = arrayRefFromStringRef(saver().save(functionBody));
   cast<SyntheticFunction>(func->function)->setBody(body);
 }
 
 bool Writer::needsPassiveInitialization(const OutputSegment *segment) {
   // If bulk memory features is supported then we can perform bss initialization
   // (via memory.fill) during `__wasm_init_memory`.
-  if (config->importMemory && !segment->requiredInBinary())
+  if (config->memoryImport.hasValue() && !segment->requiredInBinary())
     return true;
   return segment->initFlags & WASM_DATA_SEGMENT_IS_PASSIVE;
 }
 
 bool Writer::hasPassiveInitializedSegments() {
   return llvm::any_of(segments, [this](const OutputSegment *s) {
     return this->needsPassiveInitialization(s);
   });