From d6548e35b22ea138413f4079dfbc3c1e4c6a89eb Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 5 Jul 2017 20:25:08 +0000 Subject: [PATCH] [WebAssembly] Fix types for address taken functions Differential Revision: https://reviews.llvm.org/D34966 llvm-svn: 307198 --- include/llvm/MC/MCSymbolWasm.h | 14 +++++- lib/MC/WasmObjectWriter.cpp | 2 +- .../WebAssemblyTargetStreamer.cpp | 28 ++++++++--- .../MCTargetDesc/WebAssemblyTargetStreamer.h | 8 +-- .../WebAssembly/WebAssemblyAsmPrinter.cpp | 7 +-- .../WebAssembly/WebAssemblyMCInstLower.cpp | 2 - test/MC/WebAssembly/external-func-address.ll | 49 +++++++++++-------- 7 files changed, 70 insertions(+), 40 deletions(-) diff --git a/include/llvm/MC/MCSymbolWasm.h b/include/llvm/MC/MCSymbolWasm.h index 7ea89629efd..9bae6c582fa 100644 --- a/include/llvm/MC/MCSymbolWasm.h +++ b/include/llvm/MC/MCSymbolWasm.h @@ -21,6 +21,8 @@ private: std::string ModuleName; SmallVector Returns; SmallVector 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. @@ -45,15 +47,23 @@ public: const StringRef getModuleName() const { return ModuleName; } - const SmallVector &getReturns() const { return Returns; } + const SmallVector &getReturns() const { + assert(ReturnsSet); + return Returns; + } void setReturns(SmallVectorImpl &&Rets) { + ReturnsSet = true; Returns = std::move(Rets); } - const SmallVector &getParams() const { return Params; } + const SmallVector &getParams() const { + assert(ParamsSet); + return Params; + } void setParams(SmallVectorImpl &&Pars) { + ParamsSet = true; Params = std::move(Pars); } }; diff --git a/lib/MC/WasmObjectWriter.cpp b/lib/MC/WasmObjectWriter.cpp index 426f7e1bd9a..7a65b8426f8 100644 --- a/lib/MC/WasmObjectWriter.cpp +++ b/lib/MC/WasmObjectWriter.cpp @@ -153,7 +153,7 @@ struct WasmRelocationEntry { } void print(raw_ostream &Out) const { - Out << "Off=" << Offset << ", Sym=" << Symbol << ", Addend=" << Addend + Out << "Off=" << Offset << ", Sym=" << *Symbol << ", Addend=" << Addend << ", Type=" << Type << ", FixupSection=" << FixupSection; } diff --git a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp index ad59f2f4058..00bf02469bd 100644 --- a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp +++ b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp @@ -115,8 +115,8 @@ void WebAssemblyTargetAsmStreamer::emitStackPointer(uint32_t Index) { void WebAssemblyTargetAsmStreamer::emitEndFunc() { OS << "\t.endfunc\n"; } void WebAssemblyTargetAsmStreamer::emitIndirectFunctionType( - StringRef name, SmallVectorImpl &Params, SmallVectorImpl &Results) { - OS << "\t.functype\t" << name; + MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &Results) { + OS << "\t.functype\t" << Symbol->getName(); if (Results.empty()) OS << ", void"; else { @@ -171,7 +171,7 @@ void WebAssemblyTargetELFStreamer::emitIndIdx(const MCExpr *Value) { } void WebAssemblyTargetELFStreamer::emitIndirectFunctionType( - StringRef name, SmallVectorImpl &Params, SmallVectorImpl &Results) { + MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &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. } @@ -255,9 +255,25 @@ void WebAssemblyTargetWasmStreamer::emitIndIdx(const MCExpr *Value) { } void WebAssemblyTargetWasmStreamer::emitIndirectFunctionType( - StringRef name, SmallVectorImpl &Params, SmallVectorImpl &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 &Params, + SmallVectorImpl &Results) { + MCSymbolWasm *WasmSym = cast(Symbol); + if (WasmSym->isFunction()) { + // Symbol already has its arguments and result set. + return; + } + + SmallVector ValParams; + for (MVT Ty : Params) + ValParams.push_back(WebAssembly::toValType(Ty)); + + SmallVector 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) { diff --git a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h index 5ad147e5e59..102d7219a1e 100644 --- a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h +++ b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h @@ -44,7 +44,7 @@ public: /// .endfunc virtual void emitEndFunc() = 0; /// .functype - virtual void emitIndirectFunctionType(StringRef name, + virtual void emitIndirectFunctionType(MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &Results) = 0; /// .indidx @@ -69,7 +69,7 @@ public: void emitGlobal(ArrayRef Globals) override; void emitStackPointer(uint32_t Index) override; void emitEndFunc() override; - void emitIndirectFunctionType(StringRef name, + void emitIndirectFunctionType(MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &Results) override; void emitIndIdx(const MCExpr *Value) override; @@ -87,7 +87,7 @@ public: void emitGlobal(ArrayRef Globals) override; void emitStackPointer(uint32_t Index) override; void emitEndFunc() override; - void emitIndirectFunctionType(StringRef name, + void emitIndirectFunctionType(MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &Results) override; void emitIndIdx(const MCExpr *Value) override; @@ -105,7 +105,7 @@ public: void emitGlobal(ArrayRef Globals) override; void emitStackPointer(uint32_t Index) override; void emitEndFunc() override; - void emitIndirectFunctionType(StringRef name, + void emitIndirectFunctionType(MCSymbol *Symbol, SmallVectorImpl &Params, SmallVectorImpl &Results) override; void emitIndIdx(const MCExpr *Value) override; diff --git a/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp index f51585a10ca..211358ad66c 100644 --- a/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp +++ b/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp @@ -84,7 +84,7 @@ void WebAssemblyAsmPrinter::EmitEndOfAsmFile(Module &M) { SmallVector Results; SmallVector Params; ComputeSignatureVTs(F, TM, Params, Results); - getTargetStreamer()->emitIndirectFunctionType(F.getName(), Params, + getTargetStreamer()->emitIndirectFunctionType(getSymbol(&F), Params, Results); } } @@ -214,11 +214,8 @@ void WebAssemblyAsmPrinter::EmitInstruction(const MachineInstr *MI) { const MCExpr *WebAssemblyAsmPrinter::lowerConstant(const Constant *CV) { if (const GlobalValue *GV = dyn_cast(CV)) if (GV->getValueType()->isFunctionTy()) { - MCSymbol* Sym = getSymbol(GV); - if (!isa(Sym)) - cast(Sym)->setIsFunction(true); return MCSymbolRefExpr::create( - Sym, MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext); + getSymbol(GV), MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext); } return AsmPrinter::lowerConstant(CV); } diff --git a/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp b/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp index ff186eb9150..8880539804c 100644 --- a/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp +++ b/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp @@ -112,8 +112,6 @@ MCOperand WebAssemblyMCInstLower::LowerSymbolOperand(MCSymbol *Sym, MCSymbolRefExpr::VariantKind VK = IsFunc ? MCSymbolRefExpr::VK_WebAssembly_FUNCTION : MCSymbolRefExpr::VK_None; - if (!isa(Sym)) - cast(Sym)->setIsFunction(IsFunc); const MCExpr *Expr = MCSymbolRefExpr::create(Sym, VK, Ctx); diff --git a/test/MC/WebAssembly/external-func-address.ll b/test/MC/WebAssembly/external-func-address.ll index 4022b2c9bae..53da9805f98 100644 --- a/test/MC/WebAssembly/external-func-address.ll +++ b/test/MC/WebAssembly/external-func-address.ll @@ -2,24 +2,33 @@ ; 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