[WebAssembly] Fix types for address taken functions

[sbc100]

Summary

Previously we were marking these symbols as functions but setting
their params and return types resulting in the wrong signature in
the generated wasm file.

This change ensure that all Symbols that are marked as function
also their parameters and return values set.

[sbc100]

• simplify patch

[dschuff]

LGTM with a nit. In the summary, did you mean that we were *not* setting their params and return types?

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp

270 ↗

(On Diff #105122)

Might as well make this a smallvector of size 1.

[sbc100]

• use 1 element vector for results

llvm/trunk/include/llvm/MC/MCSymbolWasm.h

 
 class MCSymbolWasm : public MCSymbol {
 private:
   bool IsFunction = false;
   bool IsWeak = false;
   std::string ModuleName;
   SmallVector<wasm::ValType, 1> Returns;
   SmallVector<wasm::ValType, 4> Params;
+  bool ParamsSet = false;
+  bool ReturnsSet = false;
 
   /// An expression describing how to calculate the size of a symbol. If a
   /// symbol has no size this field will be NULL.
   const MCExpr *SymbolSize = nullptr;
 
 public:
   // Use a module name of "env" for now, for compatibility with existing tools.
   // This is temporary, and may change, as the ABI is not yet stable.
   MCSymbolWasm(const StringMapEntry<bool> *Name, bool isTemporary)
       : MCSymbol(SymbolKindWasm, Name, isTemporary),
         ModuleName("env") {}
   static bool classof(const MCSymbol *S) { return S->isWasm(); }
 
   const MCExpr *getSize() const { return SymbolSize; }
   void setSize(const MCExpr *SS) { SymbolSize = SS; }
 
   bool isFunction() const { return IsFunction; }
   void setIsFunction(bool isFunc) { IsFunction = isFunc; }
 
   bool isWeak() const { return IsWeak; }
   void setWeak(bool isWeak) { IsWeak = isWeak; }
 
   const StringRef getModuleName() const { return ModuleName; }
 
-  const SmallVector<wasm::ValType, 1> &getReturns() const { return Returns; }
+  const SmallVector<wasm::ValType, 1> &getReturns() const {
+    assert(ReturnsSet);
+    return Returns;
+  }
 
   void setReturns(SmallVectorImpl<wasm::ValType> &&Rets) {
+    ReturnsSet = true;
     Returns = std::move(Rets);
   }
 
-  const SmallVector<wasm::ValType, 4> &getParams() const { return Params; }
+  const SmallVector<wasm::ValType, 4> &getParams() const {
+    assert(ParamsSet);
+    return Params;
+  }
 
   void setParams(SmallVectorImpl<wasm::ValType> &&Pars) {
+    ParamsSet = true;
     Params = std::move(Pars);
   }
 };
 
 }  // end namespace llvm
 
 #endif // LLVM_MC_MCSYMBOLWASM_H

llvm/trunk/lib/MC/WasmObjectWriter.cpp

bool hasAddend() const {
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
       return true;
     default:
       return false;
     }
   }
 
   void print(raw_ostream &Out) const {
-    Out << "Off=" << Offset << ", Sym=" << Symbol << ", Addend=" << Addend
+    Out << "Off=" << Offset << ", Sym=" << *Symbol << ", Addend=" << Addend
         << ", Type=" << Type << ", FixupSection=" << FixupSection;
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const { print(dbgs()); }
 #endif
 };
 

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h

public:
   virtual void emitLocal(ArrayRef<MVT> Types) = 0;
   /// .globalvar
   virtual void emitGlobal(ArrayRef<wasm::Global> Globals) = 0;
   /// .stack_pointer
   virtual void emitStackPointer(uint32_t Index) = 0;
   /// .endfunc
   virtual void emitEndFunc() = 0;
   /// .functype
-  virtual void emitIndirectFunctionType(StringRef name,
+  virtual void emitIndirectFunctionType(MCSymbol *Symbol,
                                         SmallVectorImpl<MVT> &Params,
                                         SmallVectorImpl<MVT> &Results) = 0;
   /// .indidx
   virtual void emitIndIdx(const MCExpr *Value) = 0;
   /// .import_global
   virtual void emitGlobalImport(StringRef name) = 0;
 
 protected:
   void emitValueType(wasm::ValType Type);
 };
 
 /// This part is for ascii assembly output
 class WebAssemblyTargetAsmStreamer final : public WebAssemblyTargetStreamer {
   formatted_raw_ostream &OS;
 
 public:
   WebAssemblyTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for ELF object output
 class WebAssemblyTargetELFStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetELFStreamer(MCStreamer &S);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for Wasm object output
 class WebAssemblyTargetWasmStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetWasmStreamer(MCStreamer &S);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp

 
 void WebAssemblyTargetAsmStreamer::emitStackPointer(uint32_t Index) {
   OS << "\t.stack_pointer\t" << Index << '\n';
 }
 
 void WebAssemblyTargetAsmStreamer::emitEndFunc() { OS << "\t.endfunc\n"; }
 
 void WebAssemblyTargetAsmStreamer::emitIndirectFunctionType(
-    StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
-  OS << "\t.functype\t" << name;
+    MCSymbol *Symbol, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
+  OS << "\t.functype\t" << Symbol->getName();
   if (Results.empty())
     OS << ", void";
   else {
     assert(Results.size() == 1);
     OS << ", " << WebAssembly::TypeToString(Results.front());
   }
   for (auto Ty : Params)
     OS << ", " << WebAssembly::TypeToString(Ty);
void WebAssemblyTargetELFStreamer::emitEndFunc() {
   Streamer.EmitIntValue(WebAssembly::End, 1);
 }
 
 void WebAssemblyTargetELFStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
 void WebAssemblyTargetELFStreamer::emitIndirectFunctionType(
-    StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
+    MCSymbol *Symbol, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
   // Nothing to emit here. TODO: Re-design how linking works and re-evaluate
   // whether it's necessary for .o files to declare indirect function types.
 }
 
 void WebAssemblyTargetELFStreamer::emitGlobalImport(StringRef name) {
 }
 
 void WebAssemblyTargetWasmStreamer::emitParam(MCSymbol *Symbol,
void WebAssemblyTargetWasmStreamer::emitEndFunc() {
   llvm_unreachable(".end_func is not needed for direct wasm output");
 }
 
 void WebAssemblyTargetWasmStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
 void WebAssemblyTargetWasmStreamer::emitIndirectFunctionType(
-    StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
-  // Nothing to emit here. TODO: Re-design how linking works and re-evaluate
-  // whether it's necessary for .o files to declare indirect function types.
+    MCSymbol *Symbol, SmallVectorImpl<MVT> &Params,
+    SmallVectorImpl<MVT> &Results) {
+  MCSymbolWasm *WasmSym = cast<MCSymbolWasm>(Symbol);
+  if (WasmSym->isFunction()) {
+    // Symbol already has its arguments and result set.
+    return;
+  }
+
+  SmallVector<wasm::ValType, 4> ValParams;
+  for (MVT Ty : Params)
+    ValParams.push_back(WebAssembly::toValType(Ty));
+
+  SmallVector<wasm::ValType, 1> ValResults;
+  for (MVT Ty : Results)
+    ValResults.push_back(WebAssembly::toValType(Ty));
+
+  WasmSym->setParams(std::move(ValParams));
+  WasmSym->setReturns(std::move(ValResults));
+  WasmSym->setIsFunction(true);
 }
 
 void WebAssemblyTargetWasmStreamer::emitGlobalImport(StringRef name) {
 }

llvm/trunk/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

 
 void WebAssemblyAsmPrinter::EmitEndOfAsmFile(Module &M) {
   for (const auto &F : M) {
     // Emit function type info for all undefined functions
     if (F.isDeclarationForLinker() && !F.isIntrinsic()) {
       SmallVector<MVT, 4> Results;
       SmallVector<MVT, 4> Params;
       ComputeSignatureVTs(F, TM, Params, Results);
-      getTargetStreamer()->emitIndirectFunctionType(F.getName(), Params,
+      getTargetStreamer()->emitIndirectFunctionType(getSymbol(&F), Params,
                                                     Results);
     }
   }
   for (const auto &G : M.globals()) {
     if (!G.hasInitializer() && G.hasExternalLinkage()) {
       uint16_t Size = M.getDataLayout().getTypeAllocSize(G.getValueType());
       getTargetStreamer()->emitGlobalImport(G.getGlobalIdentifier());
       OutStreamer->emitELFSize(getSymbol(&G),
default: {
     break;
   }
   }
 }
 
 const MCExpr *WebAssemblyAsmPrinter::lowerConstant(const Constant *CV) {
   if (const GlobalValue *GV = dyn_cast<GlobalValue>(CV))
     if (GV->getValueType()->isFunctionTy()) {
-      MCSymbol* Sym = getSymbol(GV);
-      if (!isa<MCSymbolELF>(Sym))
-        cast<MCSymbolWasm>(Sym)->setIsFunction(true);
       return MCSymbolRefExpr::create(
-          Sym, MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext);
+          getSymbol(GV), MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext);
     }
   return AsmPrinter::lowerConstant(CV);
 }
 
 bool WebAssemblyAsmPrinter::PrintAsmOperand(const MachineInstr *MI,
                                             unsigned OpNo, unsigned AsmVariant,
                                             const char *ExtraCode,
                                             raw_ostream &OS) {

llvm/trunk/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

 }
 
 MCOperand WebAssemblyMCInstLower::LowerSymbolOperand(MCSymbol *Sym,
                                                      int64_t Offset,
                                                      bool IsFunc) const {
   MCSymbolRefExpr::VariantKind VK =
       IsFunc ? MCSymbolRefExpr::VK_WebAssembly_FUNCTION
              : MCSymbolRefExpr::VK_None;
-  if (!isa<MCSymbolELF>(Sym))
-    cast<MCSymbolWasm>(Sym)->setIsFunction(IsFunc);
 
   const MCExpr *Expr = MCSymbolRefExpr::create(Sym, VK, Ctx);
 
   if (Offset != 0) {
     if (IsFunc)
       report_fatal_error("Function addresses with offsets not supported");
     Expr =
         MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(Offset, Ctx), Ctx);

llvm/trunk/test/MC/WebAssembly/external-func-address.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 ; Verify that addresses of external functions generate correctly typed
 ; imports and relocations or type R_TABLE_INDEX_I32.
 
-declare void @f1() #1
-@ptr_to_f1 = hidden global void ()* @f1, align 4
-
+declare void @f1(i32) #1
+@ptr_to_f1 = hidden global void (i32)* @f1, align 4
 
-; CHECK:   - Type:            IMPORT
-; CHECK:     Imports:
-; CHECK:       - Module:          env
-; CHECK:         Field:           f1
-; CHECK:         Kind:            FUNCTION
-; CHECK:         SigIndex:        0
-; CHECK:   - Type:            ELEM
-; CHECK:     Segments:
-; CHECK:       - Offset:
-; CHECK:           Opcode:          I32_CONST
-; CHECK:           Value:           0
-; CHECK:         Functions:       [ 0 ]
-; CHECK:   - Type:            DATA
-; CHECK:     Relocations:
-; CHECK:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
-; CHECK:         Index:           0
-; CHECK:         Offset:          0x00000006
+; CHECK:      --- !WASM
+; CHECK-NEXT: FileHeader:      
+; CHECK-NEXT:   Version:         0x00000001
+; CHECK-NEXT: Sections:        
+; CHECK-NEXT:   - Type:            TYPE
+; CHECK-NEXT:     Signatures:      
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         ReturnType:      NORESULT
+; CHECK-NEXT:         ParamTypes:      
+; CHECK-NEXT:           - I32
+; CHECK:        - Type:            IMPORT
+; CHECK-NEXT:     Imports:
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           f1
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
+; CHECK:        - Type:            ELEM
+; CHECK-NEXT:     Segments:
+; CHECK-NEXT:       - Offset:
+; CHECK-NEXT:           Opcode:          I32_CONST
+; CHECK-NEXT:           Value:           0
+; CHECK-NEXT:         Functions:       [ 0 ]
+; CHECK:        - Type:            DATA
+; CHECK-NEXT:     Relocations:
+; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
+; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:         Offset:          0x00000006