[WebAssembly] Support weak defined symbols

[sbc100]

Summary

Model weakly defined symbols as symbols that are both
exports and imported and marked as weak. Local refereces
to the symbols refer to the import but the linker can
resolve this to the weak export if not strong symbol
is found at link time.

[dschuff]

One nit, otherwise LGTM. Don't forget to update Linking.md

test/MC/WebAssembly/weak-alias.ll

6 ↗

(On Diff #105327)

This bitcast is now a no-op, can it be removed?

[sbc100]

• rebase

[sbc100]

• remove bitcast

[mehdi_amini]

llvm/trunk/lib/MC/WasmObjectWriter.cpp

932

You dereferencing Inner after a dynamics cast, it should either be checked or you should use cast.

(I think there are other occurrences to fix in this file).

llvm/trunk/include/llvm/Object/Wasm.h

public:
   uint32_t ElementIndex;
 
   bool isWeak() const {
     return Flags & wasm::WASM_SYMBOL_FLAG_WEAK;
   }
 
   void print(raw_ostream &Out) const {
     Out << "Name=" << Name << ", Type=" << static_cast<int>(Type)
-        << ", Flags=" << Flags;
+        << ", Flags=" << Flags << " ElemIndex=" << ElementIndex;
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const { print(dbgs()); }
 #endif
 };
 
 class WasmSection {

llvm/trunk/lib/MC/WasmObjectWriter.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/StringSaver.h"
 #include <vector>
 
 using namespace llvm;
 
-#undef DEBUG_TYPE
-#define DEBUG_TYPE "reloc-info"
+#define DEBUG_TYPE "mc"
 
 namespace {
 
 // For patching purposes, we need to remember where each section starts, both
 // for patching up the section size field, and for patching up references to
 // locations within the section.
 struct SectionBookkeeping {
   // Where the size of the section is written.
class WasmObjectWriter : public MCObjectWriter {
   // Maps function symbols to the table element index space. Used
   // for TABLE_INDEX relocation types (i.e. address taken functions).
   DenseMap<const MCSymbolWasm *, uint32_t> IndirectSymbolIndices;
   // Maps function/global symbols to the function/global index space.
   DenseMap<const MCSymbolWasm *, uint32_t> SymbolIndices;
 
   DenseMap<WasmFunctionType, int32_t, WasmFunctionTypeDenseMapInfo>
       FunctionTypeIndices;
+  SmallVector<WasmFunctionType, 4> FunctionTypes;
 
   // TargetObjectWriter wrappers.
   bool is64Bit() const { return TargetObjectWriter->is64Bit(); }
   unsigned getRelocType(const MCValue &Target, const MCFixup &Fixup) const {
     return TargetObjectWriter->getRelocType(Target, Fixup);
   }
 
   void startSection(SectionBookkeeping &Section, unsigned SectionId,
private:
 
   void reset() override {
     CodeRelocations.clear();
     DataRelocations.clear();
     TypeIndices.clear();
     SymbolIndices.clear();
     IndirectSymbolIndices.clear();
     FunctionTypeIndices.clear();
+    FunctionTypes.clear();
     MCObjectWriter::reset();
   }
 
   void writeHeader(const MCAssembler &Asm);
 
   void recordRelocation(MCAssembler &Asm, const MCAsmLayout &Layout,
                         const MCFragment *Fragment, const MCFixup &Fixup,
                         MCValue Target, bool &IsPCRel,
void writeLinkingMetaDataSection(uint32_t DataSize, uint32_t DataAlignment,
                                    uint32_t StackPointerGlobal);
 
   void applyRelocations(ArrayRef<WasmRelocationEntry> Relocations,
                         uint64_t ContentsOffset);
 
   void writeRelocations(ArrayRef<WasmRelocationEntry> Relocations,
                         uint64_t HeaderSize);
   uint32_t getRelocationIndexValue(const WasmRelocationEntry &RelEntry);
+  uint32_t getFunctionType(const MCSymbolWasm& Symbol);
+  uint32_t registerFunctionType(const MCSymbolWasm& Symbol);
 };
 
 } // end anonymous namespace
 
 WasmObjectWriter::~WasmObjectWriter() {}
 
 // Return the padding size to write a 32-bit value into a 5-byte ULEB128.
 static unsigned PaddingFor5ByteULEB128(uint32_t X) {

 }
 
 uint32_t WasmObjectWriter::getRelocationIndexValue(
     const WasmRelocationEntry &RelEntry) {
   switch (RelEntry.Type) {
   case wasm::R_WEBASSEMBLY_TABLE_INDEX_SLEB:
   case wasm::R_WEBASSEMBLY_TABLE_INDEX_I32:
     if (!IndirectSymbolIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found table index space:" +
+      report_fatal_error("symbol not found table index space: " +
                          RelEntry.Symbol->getName());
     return IndirectSymbolIndices[RelEntry.Symbol];
   case wasm::R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_LEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_SLEB:
   case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
     if (!SymbolIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found function/global index space:" +
+      report_fatal_error("symbol not found function/global index space: " +
                          RelEntry.Symbol->getName());
     return SymbolIndices[RelEntry.Symbol];
   case wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB:
     if (!TypeIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found in type index space:" +
+      report_fatal_error("symbol not found in type index space: " +
                          RelEntry.Symbol->getName());
     return TypeIndices[RelEntry.Symbol];
   default:
     llvm_unreachable("invalid relocation type");
   }
 }
 
 // Apply the portions of the relocation records that we can handle ourselves
if (DataSize > 0) {
     startSection(SubSection, wasm::WASM_DATA_ALIGNMENT);
     encodeULEB128(DataAlignment, getStream());
     endSection(SubSection);
   }
 
   endSection(Section);
 }
 
+uint32_t WasmObjectWriter::getFunctionType(const MCSymbolWasm& Symbol) {
+  assert(Symbol.isFunction());
+  assert(TypeIndices.count(&Symbol));
+  return TypeIndices[&Symbol];
+}
+
+uint32_t WasmObjectWriter::registerFunctionType(const MCSymbolWasm& Symbol) {
+  assert(Symbol.isFunction());
+
+  WasmFunctionType F;
+  if (Symbol.isVariable()) {
+    const MCExpr *Expr = Symbol.getVariableValue();
+    auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr);
+    const auto *ResolvedSym = cast<MCSymbolWasm>(&Inner->getSymbol());
+    F.Returns = ResolvedSym->getReturns();
+    F.Params = ResolvedSym->getParams();
+  } else {
+    F.Returns = Symbol.getReturns();
+    F.Params = Symbol.getParams();
+  }
+
+  auto Pair =
+      FunctionTypeIndices.insert(std::make_pair(F, FunctionTypes.size()));
+  if (Pair.second)
+    FunctionTypes.push_back(F);
+  TypeIndices[&Symbol] = Pair.first->second;
+
+  DEBUG(dbgs() << "registerFunctionType: " << Symbol << " new:" << Pair.second << "\n");
+  DEBUG(dbgs() << "  -> type index: " << Pair.first->second << "\n");
+  return Pair.first->second;
+}
+
 void WasmObjectWriter::writeObject(MCAssembler &Asm,
                                    const MCAsmLayout &Layout) {
   DEBUG(dbgs() << "WasmObjectWriter::writeObject\n");
   MCContext &Ctx = Asm.getContext();
   wasm::ValType PtrType = is64Bit() ? wasm::ValType::I64 : wasm::ValType::I32;
 
   // Collect information from the available symbols.
-  SmallVector<WasmFunctionType, 4> FunctionTypes;
   SmallVector<WasmFunction, 4> Functions;
   SmallVector<uint32_t, 4> TableElems;
   SmallVector<WasmGlobal, 4> Globals;
   SmallVector<WasmImport, 4> Imports;
   SmallVector<WasmExport, 4> Exports;
   SmallVector<StringRef, 4> WeakSymbols;
   SmallPtrSet<const MCSymbolWasm *, 4> IsAddressTaken;
   unsigned NumFuncImports = 0;
case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
       break;
     default:
       break;
     }
   }
 
   // Populate the Imports set.
   for (const MCSymbol &S : Asm.symbols()) {
-    // Weak aliases don't have thier own function types.
-    if (S.isVariable())
-      continue;
-
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
-    int32_t Type;
 
-    if (WS.isFunction()) {
-      // Prepare the function's type, if we haven't seen it yet.
-      WasmFunctionType F;
-      F.Returns = WS.getReturns();
-      F.Params = WS.getParams();
-      auto Pair =
-          FunctionTypeIndices.insert(std::make_pair(F, FunctionTypes.size()));
-      if (Pair.second)
-        FunctionTypes.push_back(F);
+    if (WS.isTemporary())
+      continue;
 
-      Type = Pair.first->second;
-    } else {
-      Type = int32_t(PtrType);
-    }
+    if (WS.isFunction())
+      registerFunctionType(WS);
 
     // If the symbol is not defined in this translation unit, import it.
-    if (!WS.isTemporary() && !WS.isDefined(/*SetUsed=*/false)) {
+    if (!WS.isDefined(/*SetUsed=*/false) || WS.isVariable()) {
       WasmImport Import;
       Import.ModuleName = WS.getModuleName();
       Import.FieldName = WS.getName();
 
       if (WS.isFunction()) {
         Import.Kind = wasm::WASM_EXTERNAL_FUNCTION;
-        Import.Type = Type;
+        Import.Type = getFunctionType(WS);
         SymbolIndices[&WS] = NumFuncImports;
         ++NumFuncImports;
       } else {
         Import.Kind = wasm::WASM_EXTERNAL_GLOBAL;
-        Import.Type = Type;
+        Import.Type = int32_t(PtrType);
         SymbolIndices[&WS] = NumGlobalImports;
         ++NumGlobalImports;
       }
 
       Imports.push_back(Import);
     }
   }
 
void WasmObjectWriter::writeObject(MCAssembler &Asm,
 
   // Handle regular defined and undefined symbols.
   for (const MCSymbol &S : Asm.symbols()) {
     // Ignore unnamed temporary symbols, which aren't ever exported, imported,
     // or used in relocations.
     if (S.isTemporary() && S.getName().empty())
       continue;
 
-    // Variable references (weak references) are handled in a second pass
-    if (S.isVariable())
-      continue;
-
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
     DEBUG(dbgs() << "MCSymbol: '" << S << "'"
                  << " isDefined=" << S.isDefined() << " isExternal="
                  << S.isExternal() << " isTemporary=" << S.isTemporary()
                  << " isFunction=" << WS.isFunction()
                  << " isWeak=" << WS.isWeak()
                  << " isVariable=" << WS.isVariable() << "\n");
 
     if (WS.isWeak())
       WeakSymbols.push_back(WS.getName());
 
+    if (WS.isVariable())
+      continue;
+
     unsigned Index;
 
     if (WS.isFunction()) {
-      // Prepare the function's type, if we haven't seen it yet.
-      WasmFunctionType F;
-      F.Returns = WS.getReturns();
-      F.Params = WS.getParams();
-      auto Pair =
-          FunctionTypeIndices.insert(std::make_pair(F, FunctionTypes.size()));
-      if (Pair.second)
-        FunctionTypes.push_back(F);
-
-      int32_t Type = Pair.first->second;
-
       if (WS.isDefined(/*SetUsed=*/false)) {
         if (WS.getOffset() != 0)
           report_fatal_error(
               "function sections must contain one function each");
 
         if (WS.getSize() == 0)
           report_fatal_error(
               "function symbols must have a size set with .size");
 
         // A definition. Take the next available index.
         Index = NumFuncImports + Functions.size();
 
         // Prepare the function.
         WasmFunction Func;
-        Func.Type = Type;
+        Func.Type = getFunctionType(WS);
         Func.Sym = &WS;
         SymbolIndices[&WS] = Index;
         Functions.push_back(Func);
       } else {
-        // Should be no such thing as weak undefined symbol
-        assert(!WS.isVariable());
-
         // An import; the index was assigned above.
         Index = SymbolIndices.find(&WS)->second;
       }
 
+      DEBUG(dbgs() << "  -> function index: " << Index << "\n");
+
       // If needed, prepare the function to be called indirectly.
-      if (IsAddressTaken.count(&WS)) {
+      if (IsAddressTaken.count(&WS) != 0) {
         IndirectSymbolIndices[&WS] = TableElems.size();
+        DEBUG(dbgs() << "  -> adding to table: " << TableElems.size() << "\n");
         TableElems.push_back(Index);
       }
     } else {
       if (WS.isTemporary() && !WS.getSize())
         continue;
 
       if (!WS.isDefined(/*SetUsed=*/false))
         continue;
if (WS.isFunction()) {
 
       WasmGlobal Global;
       Global.Type = PtrType;
       Global.IsMutable = false;
       Global.HasImport = false;
       Global.InitialValue = DataSection.getSectionOffset();
       Global.ImportIndex = 0;
       SymbolIndices[&WS] = Index;
+      DEBUG(dbgs() << "  -> global index: " << Index << "\n");
       Globals.push_back(Global);
     }
 
     // If the symbol is visible outside this translation unit, export it.
-    if (WS.isExternal() && WS.isDefined(/*SetUsed=*/false)) {
+    if ((WS.isExternal() && WS.isDefined(/*SetUsed=*/false))) {
       WasmExport Export;
       Export.FieldName = WS.getName();
       Export.Index = Index;
       if (WS.isFunction())
         Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
       else
         Export.Kind = wasm::WASM_EXTERNAL_GLOBAL;
+      DEBUG(dbgs() << "  -> export " << Exports.size() << "\n");
       Exports.push_back(Export);
     }
   }
 
-  // Handle weak aliases
+  // Handle weak aliases. We need to process these in a separate pass because
+  // we need to have processed the target of the alias before the alias itself
+  // and the symbols are not necessarily ordered in this way.
   for (const MCSymbol &S : Asm.symbols()) {
     if (!S.isVariable())
       continue;
-    assert(S.isExternal());
     assert(S.isDefined(/*SetUsed=*/false));
 
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
-
-    // Find the target symbol of this weak alias
+    // Find the target symbol of this weak alias and export that index
     const MCExpr *Expr = WS.getVariableValue();
     auto *Inner = dyn_cast<MCSymbolRefExpr>(Expr);
     const auto *ResolvedSym = cast<MCSymbolWasm>(&Inner->getSymbol());
+    DEBUG(dbgs() << WS.getName() << ": weak alias of '" << *ResolvedSym << "'\n");
+    assert(SymbolIndices.count(ResolvedSym) > 0);
     uint32_t Index = SymbolIndices.find(ResolvedSym)->second;
-    DEBUG(dbgs() << "Weak alias: '" << WS << "' -> '" << ResolvedSym << "' = " << Index << "\n");
-    SymbolIndices[&WS] = Index;
+    DEBUG(dbgs() << "  -> index:" << Index << "\n");
 
     WasmExport Export;
     Export.FieldName = WS.getName();
     Export.Index = Index;
     if (WS.isFunction())
       Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
     else
       Export.Kind = wasm::WASM_EXTERNAL_GLOBAL;
-    WeakSymbols.push_back(Export.FieldName);
+    DEBUG(dbgs() << "  -> export " << Exports.size() << "\n");
     Exports.push_back(Export);
   }
 
   // Add types for indirect function calls.
   for (const WasmRelocationEntry &Fixup : CodeRelocations) {
     if (Fixup.Type != wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB)
       continue;
 
-    WasmFunctionType F;
-    F.Returns = Fixup.Symbol->getReturns();
-    F.Params = Fixup.Symbol->getParams();
-    auto Pair =
-        FunctionTypeIndices.insert(std::make_pair(F, FunctionTypes.size()));
-    if (Pair.second)
-      FunctionTypes.push_back(F);
-
-    TypeIndices[Fixup.Symbol] = Pair.first->second;
+    registerFunctionType(*Fixup.Symbol);
   }
 
   // Write out the Wasm header.
   writeHeader(Asm);
 
   writeTypeSection(FunctionTypes);
   writeImportSection(Imports);
   writeFunctionSection(Functions);

llvm/trunk/lib/Object/WasmObjectFile.cpp

 Error WasmObjectFile::parseExportSection(const uint8_t *Ptr, const uint8_t *End) {
   uint32_t Count = readVaruint32(Ptr);
   Exports.reserve(Count);
   for (uint32_t i = 0; i < Count; i++) {
     wasm::WasmExport Ex;
     Ex.Name = readString(Ptr);
     Ex.Kind = readUint8(Ptr);
     Ex.Index = readVaruint32(Ptr);
+    WasmSymbol::SymbolType ExportType;
+    bool MakeSymbol = false;
     switch (Ex.Kind) {
     case wasm::WASM_EXTERNAL_FUNCTION:
-      SymbolMap.try_emplace(Ex.Name, Symbols.size());
-      Symbols.emplace_back(Ex.Name, WasmSymbol::SymbolType::FUNCTION_EXPORT,
-                           Sections.size(), i);
-      DEBUG(dbgs() << "Adding export: " << Symbols.back()
-                   << " sym index:" << Symbols.size() << "\n");
+      ExportType = WasmSymbol::SymbolType::FUNCTION_EXPORT;
+      MakeSymbol = true;
       break;
     case wasm::WASM_EXTERNAL_GLOBAL:
-      SymbolMap.try_emplace(Ex.Name, Symbols.size());
-      Symbols.emplace_back(Ex.Name, WasmSymbol::SymbolType::GLOBAL_EXPORT,
-                           Sections.size(), i);
-      DEBUG(dbgs() << "Adding export: " << Symbols.back()
-                   << " sym index:" << Symbols.size() << "\n");
+      ExportType = WasmSymbol::SymbolType::GLOBAL_EXPORT;
+      MakeSymbol = true;
       break;
     case wasm::WASM_EXTERNAL_MEMORY:
     case wasm::WASM_EXTERNAL_TABLE:
       break;
     default:
       return make_error<GenericBinaryError>(
           "Unexpected export kind", object_error::parse_failed);
     }
+    if (MakeSymbol) {
+      auto Pair = SymbolMap.try_emplace(Ex.Name, Symbols.size());
+      if (Pair.second) {
+        Symbols.emplace_back(Ex.Name, ExportType,
+                             Sections.size(), i);
+        DEBUG(dbgs() << "Adding export: " << Symbols.back()
+                     << " sym index:" << Symbols.size() << "\n");
+      } else {
+        uint32_t SymIndex = Pair.first->second;
+        Symbols[SymIndex] = WasmSymbol(Ex.Name, ExportType, Sections.size(), i);
+        DEBUG(dbgs() << "Replacing existing symbol:  " << Symbols[SymIndex]
+                     << " sym index:" << SymIndex << "\n");
+      }
+    }
     Exports.push_back(Ex);
   }
   if (Ptr != End)
     return make_error<GenericBinaryError>("Export section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 

llvm/trunk/test/MC/WebAssembly/weak-alias.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 
 ; foo_alias() function is weak alias of function foo()
 ; Generates two exports of the same function, one of them weak
 
-@foo_alias = weak hidden alias i32 (...), bitcast (i32 ()* @foo to i32 (...)*)
+@foo_alias = weak hidden alias i32 (), i32 ()* @foo
+
+define hidden i32 @call_alias() #0 {
+entry:
+  %call = call i32 @foo_alias()
+  ret i32 %call
+}
 
 define hidden i32 @foo() #0 {
 entry:
   ret i32 0
 }
 
+
 ; CHECK:        - Type:            TYPE
 ; CHECK-NEXT:     Signatures:      
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ReturnType:      I32
 ; CHECK-NEXT:         ParamTypes:      
-; CHECK-NEXT:   - Type:            FUNCTION
-; CHECK-NEXT:     FunctionTypes:   [ 0 ]
+
+; CHECK:        - Type:            IMPORT
+; CHECK-NEXT:     Imports:         
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           foo_alias
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
+
+; CHECK:        - Type:            FUNCTION
+; CHECK-NEXT:     FunctionTypes:   [ 0, 0 ]
 
 ; CHECK:        - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
+; CHECK-NEXT:       - Name:            call_alias
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            foo_alias
 ; CHECK-NEXT:         Kind:            FUNCTION
-; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:         Index:           2
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:   
 ; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         Name:            foo_alias
+; CHECK-NEXT:       - Index:           1
+; CHECK-NEXT:         Name:            call_alias
+; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            foo
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:     DataAlignment:   0
 ; CHECK-NEXT:     SymbolInfo:      
 ; CHECK-NEXT:       - Name:            foo_alias
 ; CHECK-NEXT:         Flags:           1
 ; CHECK-NEXT: ...