[WebAssembly] Fix imported function symbol names that differ from their import names in the .o format

[sunfish]

Summary

It appears that in the current .o file format, symbol names for undefined function symbols are not included, which means that we can't have symbols where the symbol name differs from the import name.

The following patch fixes it. (It includes both llvm and lld changes, as they're closely related, but I'll split them when committing.) With this patch, the following testcase:

__attribute__((import_module("bar"), import_name("qux"))) void foo(void);

void _start(void) {
    foo();
}

compiles with `clang -nostdlib test.c --target=wasm32-unknown-unknown -Wl,--allow-undefined-file=undefined.txt

where undefined.txt contains:

foo

into a wasm that contains this import:

(import "bar" "qux" (func $foo (type 0)))

Since this requires a change to the binary form, this patch also bumps the metadata version number.

[sunfish]

• Use a symbol flag to indicate when a custom symbol name is present.
• Revert the version number bump.
• Allow globals and events to have custom symbol names too, for consistency.

[sbc100]

Looking good.

Can we can a test in the MC/WebAssembly and lld/test/wasm?

Any not do the same for data symbols?

We probably want to fail in lld if two different object import the same symbol with the different import name or import module, but that was already on my radar and I'm OK to do that as a followup.

[sunfish]

• Add MC/WebAssembly and lld tests
• Set the WASM_SYMBOL_NAME_EXPLICIT flag when a symbols name differs from its import name.

[sunfish]

In D57632#1384200, @sbc100 wrote:

Looking good.

Can we can a test in the MC/WebAssembly and lld/test/wasm?

Done.

Any not do the same for data symbols?

Data symbols don't have a corresponding import, and already include a symbol string unconditionally.

We probably want to fail in lld if two different object import the same symbol with the different import name or import module, but that was already on my radar and I'm OK to do that as a followup.

Yes, that sounds good to do.

[sbc100]

lgtm % comment about tests

Remember to update the tool-convensions/Linking.md with this new symbol flag

tools/lld/test/wasm/import-names.ll

93 ↗

(On Diff #185835)

I'd be tempted to only include IMPORT and CUSTOM sections, and to use CHECK-NEXT.

We only care about the name of function 0 too.

e.g.

; CHECK:   - Type:            CUSTOM
​; CHECK-NEXT:     Name:            name
; CHECK-NEXT:     FunctionNames:
​; CHECK-NEXT:       - Index:           0
​; CHECK-NEXT:         Name:            f0

Same for the MC test

[vitalybuka]

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/10518

FAIL: LLVM :: tools/llvm-readobj/symbols.test (29567 of 29761)
******************** TEST 'LLVM :: tools/llvm-readobj/symbols.test' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.coff-i386    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix COFF
: 'RUN: at line 3';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix ELF
: 'RUN: at line 5';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.wasm    | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test -check-prefix WASM
: 'RUN: at line 9';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols
: 'RUN: at line 10';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj --syms /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.syms
: 'RUN: at line 11';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.syms
: 'RUN: at line 12';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readobj -t /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.t
: 'RUN: at line 13';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.t
: 'RUN: at line 14';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/llvm-readelf -s -elf-output-style LLVM /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/Inputs/trivial.obj.elf-i386 > /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.lowers
: 'RUN: at line 15';   cmp /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.symbols /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/test/tools/llvm-readobj/Output/symbols.test.tmp.lowers
--
Exit Code: 1

Command Output (stderr):
--
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm/test/tools/llvm-readobj/symbols.test:107:12: error: WASM-NEXT: is not on the line after the previous match
WASM-NEXT: Module: env
           ^
<stdin>:31:8: note: 'next' match was here
 ImportModule: env
       ^
<stdin>:29:3: note: previous match ended here
 ]
  ^
<stdin>:30:1: note: non-matching line after previous match is here
 ImportName: puts
^

lld/trunk/test/wasm/import-names.ll

+; RUN: llc -filetype=obj %s -o %t.o
+; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+declare void @f0() #0
+
+define void @_start() {
+    call void @f0()
+    ret void
+}
+
+attributes #0 = { "wasm-import-module"="somewhere" "wasm-import-name"="something" }
+
+; CHECK:        - Type:            IMPORT
+; CHECK-NEXT:     Imports:
+; CHECK-NEXT:       - Module:          somewhere
+; CHECK-NEXT:         Field:           something
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
+
+; CHECK:        - Type:            CUSTOM
+; CHECK-NEXT:     Name:            name
+; CHECK-NEXT:     FunctionNames:
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         Name:            f0

lld/trunk/wasm/Driver.cpp

if (auto *LazySym = dyn_cast<LazySymbol>(Sym))
     LazySym->fetch();
 
   return Sym;
 }
 
 static UndefinedGlobal *
 createUndefinedGlobal(StringRef Name, llvm::wasm::WasmGlobalType *Type) {
   auto *Sym =
-      cast<UndefinedGlobal>(Symtab->addUndefinedGlobal(Name, 0, nullptr, Type));
+      cast<UndefinedGlobal>(Symtab->addUndefinedGlobal(Name, Name,
+                                                       DefaultModule, 0,
+                                                       nullptr, Type));
   Config->AllowUndefinedSymbols.insert(Sym->getName());
   Sym->IsUsedInRegularObj = true;
   return Sym;
 }
 
 // Create ABI-defined synthetic symbols
 static void createSyntheticSymbols() {
   static WasmSignature NullSignature = {{}, {}};

lld/trunk/wasm/InputFiles.cpp

 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &Sym) {
   StringRef Name = Sym.Info.Name;
   uint32_t Flags = Sym.Info.Flags;
 
   switch (Sym.Info.Kind) {
   case WASM_SYMBOL_TYPE_FUNCTION:
-    return Symtab->addUndefinedFunction(Name, Sym.Info.Module, Flags, this,
+    return Symtab->addUndefinedFunction(Name, Sym.Info.ImportName,
+                                        Sym.Info.ImportModule, Flags, this,
                                         Sym.Signature);
   case WASM_SYMBOL_TYPE_DATA:
     return Symtab->addUndefinedData(Name, Flags, this);
   case WASM_SYMBOL_TYPE_GLOBAL:
-    return Symtab->addUndefinedGlobal(Name, Flags, this, Sym.GlobalType);
+    return Symtab->addUndefinedGlobal(Name, Sym.Info.ImportName,
+                                      Sym.Info.ImportModule, Flags, this,
+                                      Sym.GlobalType);
   case WASM_SYMBOL_TYPE_SECTION:
     llvm_unreachable("section symbols cannot be undefined");
   }
   llvm_unreachable("unknown symbol kind");
 }
 
 void ArchiveFile::parse() {
   // Parse a MemoryBufferRef as an archive file.
static Symbol *createBitcodeSymbol(const lto::InputFile::Symbol &ObjSym,
                                    BitcodeFile &F) {
   StringRef Name = Saver.save(ObjSym.getName());
 
   uint32_t Flags = ObjSym.isWeak() ? WASM_SYMBOL_BINDING_WEAK : 0;
   Flags |= mapVisibility(ObjSym.getVisibility());
 
   if (ObjSym.isUndefined()) {
     if (ObjSym.isExecutable())
-      return Symtab->addUndefinedFunction(Name, DefaultModule, Flags, &F, nullptr);
+      return Symtab->addUndefinedFunction(Name, Name, DefaultModule, Flags, &F,
+                                          nullptr);
     return Symtab->addUndefinedData(Name, Flags, &F);
   }
 
   if (ObjSym.isExecutable())
     return Symtab->addDefinedFunction(Name, Flags, &F, nullptr);
   return Symtab->addDefinedData(Name, Flags, &F, nullptr, 0, 0);
 }
 

lld/trunk/wasm/LTO.cpp

 }
 
 BitcodeCompiler::BitcodeCompiler() : LTOObj(createLTO()) {}
 
 BitcodeCompiler::~BitcodeCompiler() = default;
 
 static void undefine(Symbol *S) {
   if (auto F = dyn_cast<DefinedFunction>(S))
-    replaceSymbol<UndefinedFunction>(F, F->getName(), DefaultModule, 0,
+    replaceSymbol<UndefinedFunction>(F, F->getName(), F->getName(),
+                                     DefaultModule, 0,
                                      F->getFile(), F->Signature);
   else if (isa<DefinedData>(S))
     replaceSymbol<UndefinedData>(S, S->getName(), 0, S->getFile());
   else
     llvm_unreachable("unexpected symbol kind");
 }
 
 void BitcodeCompiler::add(BitcodeFile &F) {

lld/trunk/wasm/SymbolTable.h

public:
   Symbol *addDefinedData(StringRef Name, uint32_t Flags, InputFile *File,
                          InputSegment *Segment, uint32_t Address,
                          uint32_t Size);
   Symbol *addDefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
                            InputGlobal *G);
   Symbol *addDefinedEvent(StringRef Name, uint32_t Flags, InputFile *File,
                           InputEvent *E);
 
-  Symbol *addUndefinedFunction(StringRef Name, StringRef Module, uint32_t Flags,
+  Symbol *addUndefinedFunction(StringRef Name, StringRef ImportName,
+                               StringRef ImportModule, uint32_t Flags,
                                InputFile *File, const WasmSignature *Signature);
   Symbol *addUndefinedData(StringRef Name, uint32_t Flags, InputFile *File);
-  Symbol *addUndefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
-                             const WasmGlobalType *Type);
+  Symbol *addUndefinedGlobal(StringRef Name, StringRef ImportName,
+                             StringRef ImportModule,  uint32_t Flags,
+                             InputFile *File, const WasmGlobalType *Type);
 
   void addLazy(ArchiveFile *F, const llvm::object::Archive::Symbol *Sym);
 
   bool addComdat(StringRef Name);
 
   DefinedData *addSyntheticDataSymbol(StringRef Name, uint32_t Flags);
   DefinedGlobal *addSyntheticGlobal(StringRef Name, uint32_t Flags,
                                     InputGlobal *Global);

lld/trunk/wasm/SymbolTable.cpp

Symbol *SymbolTable::addDefinedEvent(StringRef Name, uint32_t Flags,
 
   checkEventType(S, File, &Event->getType(), &Event->Signature);
 
   if (shouldReplace(S, File, Flags))
     replaceSymbol<DefinedEvent>(S, Name, Flags, File, Event);
   return S;
 }
 
-Symbol *SymbolTable::addUndefinedFunction(StringRef Name, StringRef Module,
+Symbol *SymbolTable::addUndefinedFunction(StringRef Name, StringRef ImportName,
+                                          StringRef ImportModule,
                                           uint32_t Flags, InputFile *File,
                                           const WasmSignature *Sig) {
   LLVM_DEBUG(dbgs() << "addUndefinedFunction: " << Name <<
              " [" << (Sig ? toString(*Sig) : "none") << "]\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted)
-    replaceSymbol<UndefinedFunction>(S, Name, Module, Flags, File, Sig);
+    replaceSymbol<UndefinedFunction>(S, Name, ImportName, ImportModule, Flags,
+                                     File, Sig);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else
     checkFunctionType(S, File, Sig);
 
   return S;
 }
 
if (WasInserted)
     replaceSymbol<UndefinedData>(S, Name, Flags, File);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else if (S->isDefined())
     checkDataType(S, File);
   return S;
 }
 
-Symbol *SymbolTable::addUndefinedGlobal(StringRef Name, uint32_t Flags,
+Symbol *SymbolTable::addUndefinedGlobal(StringRef Name, StringRef ImportName,
+                                        StringRef ImportModule, uint32_t Flags,
                                         InputFile *File,
                                         const WasmGlobalType *Type) {
   LLVM_DEBUG(dbgs() << "addUndefinedGlobal: " << Name << "\n");
 
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name, File);
 
   if (WasInserted)
-    replaceSymbol<UndefinedGlobal>(S, Name, Flags, File, Type);
+    replaceSymbol<UndefinedGlobal>(S, Name, ImportName, ImportModule, Flags,
+                                   File, Type);
   else if (auto *Lazy = dyn_cast<LazySymbol>(S))
     Lazy->fetch();
   else if (S->isDefined())
     checkGlobalType(S, File, Type);
   return S;
 }
 
 void SymbolTable::addLazy(ArchiveFile *File, const Archive::Symbol *Sym) {

lld/trunk/wasm/Symbols.h

static bool classof(const Symbol *S) {
     return S->kind() == DefinedFunctionKind;
   }
 
   InputFunction *Function;
 };
 
 class UndefinedFunction : public FunctionSymbol {
 public:
-  UndefinedFunction(StringRef Name, StringRef Module, uint32_t Flags,
+  UndefinedFunction(StringRef Name, StringRef ImportName,
+                    StringRef ImportModule, uint32_t Flags,
                     InputFile *File = nullptr,
                     const WasmSignature *Type = nullptr)
       : FunctionSymbol(Name, UndefinedFunctionKind, Flags, File, Type),
-        Module(Module) {}
+        ImportName(ImportName), ImportModule(ImportModule) {}
 
   static bool classof(const Symbol *S) {
     return S->kind() == UndefinedFunctionKind;
   }
 
-  StringRef Module;
+  StringRef ImportName;
+  StringRef ImportModule;
 };
 
 class SectionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) { return S->kind() == SectionKind; }
 
   SectionSymbol(StringRef Name, uint32_t Flags, const InputSection *S,
                 InputFile *F = nullptr)
static bool classof(const Symbol *S) {
     return S->kind() == DefinedGlobalKind;
   }
 
   InputGlobal *Global;
 };
 
 class UndefinedGlobal : public GlobalSymbol {
 public:
-  UndefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File = nullptr,
+  UndefinedGlobal(StringRef Name, StringRef ImportName, StringRef ImportModule,
+                  uint32_t Flags, InputFile *File = nullptr,
                   const WasmGlobalType *Type = nullptr)
-      : GlobalSymbol(Name, UndefinedGlobalKind, Flags, File, Type) {}
+      : GlobalSymbol(Name, UndefinedGlobalKind, Flags, File, Type),
+        ImportName(ImportName), ImportModule(ImportModule) {}
 
   static bool classof(const Symbol *S) {
     return S->kind() == UndefinedGlobalKind;
   }
+
+  StringRef ImportName;
+  StringRef ImportModule;
 };
 
 // Wasm events are features that suspend the current execution and transfer the
 // control flow to a corresponding handler. Currently the only supported event
 // kind is exceptions.
 //
 // Event tags are values to distinguish different events. For exceptions, they
 // can be used to distinguish different language's exceptions, i.e., all C++

lld/trunk/wasm/Writer.cpp

if (Config->ImportTable) {
     Import.Kind = WASM_EXTERNAL_TABLE;
     Import.Table.ElemType = WASM_TYPE_FUNCREF;
     Import.Table.Limits = {0, TableSize, 0};
     writeImport(OS, Import);
   }
 
   for (const Symbol *Sym : ImportedSymbols) {
     WasmImport Import;
-    if (auto *F = dyn_cast<UndefinedFunction>(Sym))
-      Import.Module = F->Module;
-    else
+    if (auto *F = dyn_cast<UndefinedFunction>(Sym)) {
+      Import.Field = F->ImportName;
+      Import.Module = F->ImportModule;
+    } else if (auto *G = dyn_cast<UndefinedGlobal>(Sym)) {
+      Import.Field = G->ImportName;
+      Import.Module = G->ImportModule;
+    } else {
+      Import.Field = Sym->getName();
       Import.Module = DefaultModule;
+    }
 
-    Import.Field = Sym->getName();
     if (auto *FunctionSym = dyn_cast<FunctionSymbol>(Sym)) {
       Import.Kind = WASM_EXTERNAL_FUNCTION;
       Import.SigIndex = lookupType(*FunctionSym->Signature);
     } else if (auto *GlobalSym = dyn_cast<GlobalSymbol>(Sym)) {
       Import.Kind = WASM_EXTERNAL_GLOBAL;
       Import.Global = *GlobalSym->getGlobalType();
     } else {
       auto *EventSym = cast<EventSymbol>(Sym);
static uint32_t getWasmFlags(const Symbol *Sym) {
   if (Sym->isLocal())
     Flags |= WASM_SYMBOL_BINDING_LOCAL;
   if (Sym->isWeak())
     Flags |= WASM_SYMBOL_BINDING_WEAK;
   if (Sym->isHidden())
     Flags |= WASM_SYMBOL_VISIBILITY_HIDDEN;
   if (Sym->isUndefined())
     Flags |= WASM_SYMBOL_UNDEFINED;
+  if (auto *F = dyn_cast<UndefinedFunction>(Sym)) {
+    if (F->getName() != F->ImportName)
+      Flags |= WASM_SYMBOL_EXPLICIT_NAME;
+  } else if (auto *G = dyn_cast<UndefinedGlobal>(Sym)) {
+    if (G->getName() != G->ImportName)
+      Flags |= WASM_SYMBOL_EXPLICIT_NAME;
+  }
   return Flags;
 }
 
 // Some synthetic sections (e.g. "name" and "linking") have subsections.
 // Just like the synthetic sections themselves these need to be created before
 // they can be written out (since they are preceded by their length). This
 // class is used to create subsections and then write them into the stream
 // of the parent section.
for (const Symbol *Sym : SymtabEntries) {
       WasmSymbolType Kind = Sym->getWasmType();
       uint32_t Flags = getWasmFlags(Sym);
 
       writeU8(Sub.OS, Kind, "sym kind");
       writeUleb128(Sub.OS, Flags, "sym flags");
 
       if (auto *F = dyn_cast<FunctionSymbol>(Sym)) {
         writeUleb128(Sub.OS, F->getFunctionIndex(), "index");
-        if (Sym->isDefined())
+        if (Sym->isDefined() ||
+            (Flags & WASM_SYMBOL_EXPLICIT_NAME) != 0)
           writeStr(Sub.OS, Sym->getName(), "sym name");
       } else if (auto *G = dyn_cast<GlobalSymbol>(Sym)) {
         writeUleb128(Sub.OS, G->getGlobalIndex(), "index");
-        if (Sym->isDefined())
+        if (Sym->isDefined() ||
+            (Flags & WASM_SYMBOL_EXPLICIT_NAME) != 0)
           writeStr(Sub.OS, Sym->getName(), "sym name");
       } else if (auto *E = dyn_cast<EventSymbol>(Sym)) {
         writeUleb128(Sub.OS, E->getEventIndex(), "index");
-        if (Sym->isDefined())
+        if (Sym->isDefined() ||
+            (Flags & WASM_SYMBOL_EXPLICIT_NAME) != 0)
           writeStr(Sub.OS, Sym->getName(), "sym name");
       } else if (isa<DataSymbol>(Sym)) {
         writeStr(Sub.OS, Sym->getName(), "sym name");
         if (auto *DataSym = dyn_cast<DefinedData>(Sym)) {
           writeUleb128(Sub.OS, DataSym->getOutputSegmentIndex(), "index");
           writeUleb128(Sub.OS, DataSym->getOutputSegmentOffset(),
                        "data offset");
           writeUleb128(Sub.OS, DataSym->getSize(), "data size");