[WebAssembly] Generate imports for undefined data as well as functions

[sbc100]

Summary

If the final linked binary contains undefined data symbols
generate inputs for these, just like we do for undefined
functions

[sbc100]

.

[ncw]

Hmm, does this work? I mean I can see that it generates an imported global of course, but it doesn't do anything. The code that actually references the import is still going to have 0 written in as the value of the relocation!

The "fake globals" that are currently exported for defined data are at least usable in JavaScript, as a way of finding out the address to write to when you want to modify the variable. Is there a similar use (at this stage) for these imported globals?

Maybe hold off merging this until the discussion has come up with a full solution to the problem (assuming that this is the same as https://github.com/WebAssembly/tool-conventions/issues/48).

[sbc100]

You are right, the imported globals are never used.

And yes this was as first step to solving https://github.com/WebAssembly/tool-conventions/issues/48.

And yes I agree we probably don't want to land this without the imports actually being used.

And yes, sadly we do have use case for this, in emscripten. I injects the addresses of globals in this way.. something I only just discovered :(

[sbc100]

• rebase

test/wasm/undefined.ll

 ; RUN: llc -filetype=obj %s -o %t.o
 ; RUN: wasm-ld --allow-undefined -o %t.wasm %t.o
 
 ; Fails due to undefined 'foo' and also 'baz'
-; RUN: not wasm-ld --undefined=baz -o %t.wasm %t.o 2>&1 | FileCheck %s
-; CHECK: error: {{.*}}.o: undefined symbol: foo
-; CHECK: error: undefined symbol: baz
+; RUN: not wasm-ld --undefined=baz -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
+; CHECK-ERROR: error: {{.*}}.o: undefined symbol: foo
+; CHECK-ERROR: error: {{.*}}.o: undefined symbol: bar
+; CHECK-ERROR: error: undefined symbol: baz
 
 ; Succeeds if we pass a file containing 'foo' as --allow-undefined-file.
 ; RUN: echo 'foo' > %t.txt
+; RUN: echo 'bar' >> %t.txt
 ; RUN: wasm-ld --allow-undefined-file=%t.txt -o %t.wasm %t.o
+; RUN: obj2yaml %t.wasm | FileCheck %s
 
 ; Succeeds even if a missing symbol is added via --export
 ; RUN: wasm-ld --allow-undefined --export=xxx -o %t.wasm %t.o
 
 target triple = "wasm32-unknown-unknown"
 
-; Takes the address of the external foo() resulting in undefined external
-@bar = hidden local_unnamed_addr global i8* bitcast (i32 ()* @foo to i8*), align 4
+; Takes the address of the external function foo()
+@addr_func = local_unnamed_addr global i8* bitcast (i32 ()* @foo to i8*), align 4
+
+; Take the address of the external_data
+@addr_data = local_unnamed_addr global i32* @bar, align 4
 
 declare i32 @foo() #0
+@bar = external global i32
 
-define hidden void @_start() local_unnamed_addr #0 {
+define void @_start() local_unnamed_addr #0 {
 entry:
     ret void
 }
+
+; CHECK:        - Type:            IMPORT
+; CHECK-NEXT:     Imports:         
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           foo
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           bar
+; CHECK-NEXT:         Kind:            GLOBAL
+; CHECK-NEXT:         GlobalType:      I32
+; CHECK-NEXT:         GlobalMutable:   false

wasm/Symbols.h

public:
 
   void setHidden(bool IsHidden);
 
   // Get/set the index in the output symbol table.  This is only used for
   // relocatable output.
   uint32_t getOutputSymbolIndex() const;
   void setOutputSymbolIndex(uint32_t Index);
 
+  // Get/set the global index
+  uint32_t getGlobalIndex() const;
+  void setGlobalIndex(uint32_t Index);
+  bool hasGlobalIndex() const;
+
   WasmSymbolType getWasmType() const;
 
 protected:
   Symbol(StringRef Name, Kind K, uint32_t Flags, InputFile *F)
       : Name(Name), SymbolKind(K), Flags(Flags), File(F),
         Referenced(!Config->GcSections) {}
 
   StringRef Name;
   Kind SymbolKind;
   uint32_t Flags;
   InputFile *File;
   uint32_t OutputSymbolIndex = INVALID_INDEX;
+  uint32_t GlobalIndex = INVALID_INDEX;
   bool Referenced = false;
 };
 
 class FunctionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) {
     return S->kind() == DefinedFunctionKind ||
            S->kind() == UndefinedFunctionKind;

 class GlobalSymbol : public Symbol {
 public:
   static bool classof(const Symbol *S) {
     return S->kind() == DefinedGlobalKind || S->kind() == UndefinedGlobalKind;
   }
 
   const WasmGlobalType *getGlobalType() const { return GlobalType; }
 
-  // Get/set the global index
-  uint32_t getGlobalIndex() const;
-  void setGlobalIndex(uint32_t Index);
-  bool hasGlobalIndex() const;
-
 protected:
   GlobalSymbol(StringRef Name, Kind K, uint32_t Flags, InputFile *F,
                const WasmGlobalType *GlobalType)
       : Symbol(Name, K, Flags, F), GlobalType(GlobalType) {}
 
   // Explicit function type, needed for undefined or synthetic functions only.
   // For regular defined globals this information comes from the InputChunk.
   const WasmGlobalType *GlobalType;
-  uint32_t GlobalIndex = INVALID_INDEX;
 };
 
 class DefinedGlobal : public GlobalSymbol {
 public:
   DefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
                 InputGlobal *Global);
 
   static bool classof(const Symbol *S) {

wasm/Symbols.cpp

 }
 
 void Symbol::setOutputSymbolIndex(uint32_t Index) {
   DEBUG(dbgs() << "setOutputSymbolIndex " << Name << " -> " << Index << "\n");
   assert(OutputSymbolIndex == INVALID_INDEX);
   OutputSymbolIndex = Index;
 }
 
+uint32_t Symbol::getGlobalIndex() const {
+  if (auto *F = dyn_cast<DefinedGlobal>(this))
+    return F->Global->getGlobalIndex();
+  assert(GlobalIndex != INVALID_INDEX);
+  return GlobalIndex;
+}
+
+void Symbol::setGlobalIndex(uint32_t Index) {
+  DEBUG(dbgs() << "setGlobalIndex " << Name << " -> " << Index << "\n");
+  assert(GlobalIndex == INVALID_INDEX);
+  GlobalIndex = Index;
+}
+
+bool Symbol::hasGlobalIndex() const {
+  if (auto *F = dyn_cast<DefinedGlobal>(this))
+    return F->Global->hasGlobalIndex();
+  return GlobalIndex != INVALID_INDEX;
+}
+
 bool Symbol::isWeak() const {
   return (Flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK;
 }
 
 bool Symbol::isLocal() const {
   return (Flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_LOCAL;
 }
 
uint32_t DefinedData::getOutputSegmentOffset() const {
   return Segment->OutputSegmentOffset + Offset;
 }
 
 uint32_t DefinedData::getOutputSegmentIndex() const {
   DEBUG(dbgs() << "getOutputSegmentIndex: " << getName() << "\n");
   return Segment->OutputSeg->Index;
 }
 
-uint32_t GlobalSymbol::getGlobalIndex() const {
-  if (auto *F = dyn_cast<DefinedGlobal>(this))
-    return F->Global->getGlobalIndex();
-  assert(GlobalIndex != INVALID_INDEX);
-  return GlobalIndex;
-}
-
-void GlobalSymbol::setGlobalIndex(uint32_t Index) {
-  DEBUG(dbgs() << "setGlobalIndex " << Name << " -> " << Index << "\n");
-  assert(GlobalIndex == INVALID_INDEX);
-  GlobalIndex = Index;
-}
-
-bool GlobalSymbol::hasGlobalIndex() const {
-  if (auto *F = dyn_cast<DefinedGlobal>(this))
-    return F->Global->hasGlobalIndex();
-  return GlobalIndex != INVALID_INDEX;
-}
-
 DefinedGlobal::DefinedGlobal(StringRef Name, uint32_t Flags, InputFile *File,
                              InputGlobal *Global)
     : GlobalSymbol(Name, DefinedGlobalKind, Flags, File,
                    Global ? &Global->getType() : nullptr),
       Global(Global) {}
 
 uint32_t SectionSymbol::getOutputSectionIndex() const {
   DEBUG(dbgs() << "getOutputSectionIndex: " << getName() << "\n");

wasm/Writer.cpp

void Writer::createImportSection() {
 
   for (const Symbol *Sym : ImportedSymbols) {
     WasmImport Import;
     Import.Module = "env";
     Import.Field = Sym->getName();
     if (auto *FunctionSym = dyn_cast<FunctionSymbol>(Sym)) {
       Import.Kind = WASM_EXTERNAL_FUNCTION;
       Import.SigIndex = lookupType(*FunctionSym->getFunctionType());
-    } else {
-      auto *GlobalSym = cast<GlobalSymbol>(Sym);
+    } else if (auto *GlobalSym = dyn_cast<GlobalSymbol>(Sym)) {
       Import.Kind = WASM_EXTERNAL_GLOBAL;
       Import.Global = *GlobalSym->getGlobalType();
+    } else {
+      assert(isa<DataSymbol>(Sym));
+      Import.Kind = WASM_EXTERNAL_GLOBAL;
+      Import.Global = {WASM_TYPE_I32, false};
     }
     writeImport(OS, Import);
   }
 }
 
 void Writer::createTypeSection() {
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_TYPE);
   raw_ostream &OS = Section->getStream();
for (OutputSection *S : OutputSections) {
     FileSize += S->getSize();
   }
 }
 
 void Writer::calculateImports() {
   for (Symbol *Sym : Symtab->getSymbols()) {
     if (!Sym->isUndefined())
       continue;
-    if (isa<DataSymbol>(Sym))
+    // For data symbols we don't use imports in the relocatable output, instead
+    // these are in the explict symbol table.
+    if (isa<DataSymbol>(Sym) && Config->Relocatable)
       continue;
     if (Sym->isWeak() && !Config->Relocatable)
       continue;
     if (!Sym->isLive())
       continue;
 
     DEBUG(dbgs() << "import: " << Sym->getName() << "\n");
     ImportedSymbols.emplace_back(Sym);
     if (auto *F = dyn_cast<FunctionSymbol>(Sym))
       F->setFunctionIndex(NumImportedFunctions++);
+    else if (auto *G = dyn_cast<GlobalSymbol>(Sym))
+      G->setGlobalIndex(NumImportedGlobals++);
     else
-      cast<GlobalSymbol>(Sym)->setGlobalIndex(NumImportedGlobals++);
+      cast<DataSymbol>(Sym)->setGlobalIndex(NumImportedGlobals++);
   }
 }
 
 void Writer::calculateExports() {
   if (Config->Relocatable)
     return;
 
   if (!Config->Relocatable && !Config->ImportMemory)