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

[sunfish]

Summary

This 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 based on https://reviews.llvm.org/D131376, with the main
difference being that it only supports exporting memory by one name.

I know the discussion in D131376 was toward looking into ways to do this by
declaring memories in the source code, and that still makes sense to look into
for more advanced use cases, but for the common case, we don't have any
other reason to declare memory explicitly; we just want lld to use a different
name from the one hard-coded into it, so this adds a way to do that.

[sbc100]

Seems reasonable. Does the same thing need to be don't for the indirect function table?

lld/wasm/Driver.cpp

538

why not use defaultModule and memoryName here?

544

If you use memoryName below maybe change this to default to exporting memory under its default name.?

546

Why not use memoryName here?

lld/wasm/SyntheticSections.cpp

237

Does llvm::Optional no coerce to boolean automatically here?

[sunfish]

• Address review feedback

[sbc100]

lld/wasm/Driver.cpp

546

Does this change the default linker behaviour to export the memory (when no flags are specified)? I didn't realize that was the default behavior.

[sunfish]

lld/wasm/Driver.cpp

546

Yes, currently wasm-ld exports the memory by default. This patch preserves that default.

[sbc100]

lgtm.

I wonder if we should consider changing the default to not export or import the memory?

[fmayer]

This broke our build bots: https://lab.llvm.org/buildbot/#/builders/19/builds/13332/steps/25/logs/stdio

FAILED: tools/lld/wasm/CMakeFiles/lldWasm.dir/Driver.cpp.o 
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/clang_build/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lld/wasm -I/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm -I/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/include -Itools/lld/include -Iinclude -I/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG    -fno-exceptions -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/lld/wasm/CMakeFiles/lldWasm.dir/Driver.cpp.o -MF tools/lld/wasm/CMakeFiles/lldWasm.dir/Driver.cpp.o.d -o tools/lld/wasm/CMakeFiles/lldWasm.dir/Driver.cpp.o -c /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm/Driver.cpp
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm/Driver.cpp:533:30: error: 'hasValue' is deprecated: Use has_value instead. [-Werror,-Wdeprecated-declarations]
    if (config->memoryExport.hasValue()) {
                             ^~~~~~~~
                             has_value
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/ADT/Optional.h:325:3: note: 'hasValue' has been explicitly marked deprecated here
  LLVM_DEPRECATED("Use has_value instead.", "has_value")
  ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/Support/Compiler.h:143:50: note: expanded from macro 'LLVM_DEPRECATED'
#define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
                                                 ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm/Driver.cpp:536:31: error: 'hasValue' is deprecated: Use has_value instead. [-Werror,-Wdeprecated-declarations]
    if (!config->memoryImport.hasValue()) {
                              ^~~~~~~~
                              has_value
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/ADT/Optional.h:325:3: note: 'hasValue' has been explicitly marked deprecated here
  LLVM_DEPRECATED("Use has_value instead.", "has_value")
  ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/Support/Compiler.h:143:50: note: expanded from macro 'LLVM_DEPRECATED'
#define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
                                                 ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm/Driver.cpp:545:29: error: 'hasValue' is deprecated: Use has_value instead. [-Werror,-Wdeprecated-declarations]
  if (!config->memoryExport.hasValue() && !config->memoryImport.hasValue()) {
                            ^~~~~~~~
                            has_value
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/ADT/Optional.h:325:3: note: 'hasValue' has been explicitly marked deprecated here
  LLVM_DEPRECATED("Use has_value instead.", "has_value")
  ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/Support/Compiler.h:143:50: note: expanded from macro 'LLVM_DEPRECATED'
#define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
                                                 ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/lld/wasm/Driver.cpp:545:65: error: 'hasValue' is deprecated: Use has_value instead. [-Werror,-Wdeprecated-declarations]
  if (!config->memoryExport.hasValue() && !config->memoryImport.hasValue()) {
                                                                ^~~~~~~~
                                                                has_value
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/ADT/Optional.h:325:3: note: 'hasValue' has been explicitly marked deprecated here
  LLVM_DEPRECATED("Use has_value instead.", "has_value")
  ^
/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/Support/Compiler.h:143:50: note: expanded from macro 'LLVM_DEPRECATED'
#define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
                                                 ^
4 errors generated.

[fmayer]

In D135898#3897954, @fmayer wrote:

This broke our build bots: https://lab.llvm.org/buildbot/#/builders/19/builds/13332/steps/25/logs/stdio

I'll submit a fix for this.

[fmayer]

In D135898#3897973, @fmayer wrote:

In D135898#3897954, @fmayer wrote:

This broke our build bots: https://lab.llvm.org/buildbot/#/builders/19/builds/13332/steps/25/logs/stdio

I'll submit a fix for this.

Submitted rG2390bb2347703bf500333fcd4d8c1b513a6e1740.

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 -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:            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=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:            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::Optional<llvm::StringRef> memoryExport;
   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>(defaultModule, memoryName);
+  } else {
+    config->memoryImport =
+        llvm::Optional<std::pair<llvm::StringRef, llvm::StringRef>>();
+  }
+
+  if (args.hasArg(OPT_export_memory_with_name)) {
+    config->memoryExport =
+        args.getLastArgValue(OPT_export_memory_with_name);
+  } else if (args.hasArg(OPT_export_memory)) {
+    config->memoryExport = memoryName;
+  } else {
+    config->memoryExport = llvm::Optional<llvm::StringRef>();
+  }
+
   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->memoryExport.hasValue()) {
+      error("--export-memory is incompatible with --shared");
+    }
+    if (!config->memoryImport.hasValue()) {
+      config->memoryImport =
+          std::pair<llvm::StringRef, llvm::StringRef>("env", "memory");
+    }
     config->importUndefined = true;
   }
+
+  // If neither export-memory nor import-memory is specified, default to
+  // exporting memory as '"memory"'.
+  if (!config->memoryExport.hasValue() && !config->memoryImport.hasValue()) {
+    config->memoryExport = "memory";
+  }
 }
 
 // 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)
     error("--compress-relocations is incompatible with output debug"
           " information. Please pass --strip-debug or --strip-all");

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/Symbols.h

 // Shared string constants
 
 // The default module name to use for symbol imports.
 extern const char *defaultModule;
 
 // The name under which to import or export the wasm table.
 extern const char *functionTableName;
 
+// The name under which to import or export the wasm memory.
+extern const char *memoryName;
+
 using llvm::wasm::WasmSymbolType;
 
 class InputFile;
 class InputChunk;
 class InputSegment;
 class InputFunction;
 class InputGlobal;
 class InputTag;

lld/wasm/Symbols.cpp

void printTraceSymbol(Symbol *sym) {
   else
     s = ": definition of ";
 
   message(toString(sym->getFile()) + s + sym->getName());
 }
 
 const char *defaultModule = "env";
 const char *functionTableName = "__indirect_function_table";
+const char *memoryName = "memory";
 
 } // namespace wasm
 } // namespace lld

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->symbols()) {
     out.importSec->addImport(sym);
   }
 }
 
 void Writer::calculateExports() {
   if (config->relocatable)
     return;
 
-  if (!config->relocatable && !config->importMemory)
+  if (!config->relocatable && config->memoryExport.hasValue()) {
     out.exportSec->exports.push_back(
-        WasmExport{"memory", WASM_EXTERNAL_MEMORY, 0});
+        WasmExport{*config->memoryExport, WASM_EXTERNAL_MEMORY, 0});
+  }
 
   unsigned globalIndex =
       out.importSec->getNumImportedGlobals() + out.globalSec->numGlobals();
 
   for (Symbol *sym : symtab->symbols()) {
     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);
   });