From 2b0a4f95fdb2f31817434a045e51572bd479c735 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Thu, 29 Oct 2020 19:41:59 -0700 Subject: [PATCH] [MC] Add SMLoc to MCStreamer::emitSymbolAttribute and report changed binding warnings/errors for ELF --- include/llvm/MC/MCELFStreamer.h | 3 ++- include/llvm/MC/MCStreamer.h | 4 ++-- include/llvm/MC/MCWasmStreamer.h | 3 ++- include/llvm/MC/MCWinCOFFStreamer.h | 3 ++- include/llvm/MC/MCXCOFFStreamer.h | 3 ++- lib/MC/MCAsmStreamer.cpp | 5 +++-- lib/MC/MCELFStreamer.cpp | 15 ++++++++------- lib/MC/MCMachOStreamer.cpp | 7 ++++--- lib/MC/MCNullStreamer.cpp | 3 +-- lib/MC/MCParser/AsmParser.cpp | 2 +- lib/MC/MCParser/ELFAsmParser.cpp | 4 ++-- lib/MC/MCWasmStreamer.cpp | 3 ++- lib/MC/MCWinCOFFStreamer.cpp | 4 ++-- lib/MC/MCXCOFFStreamer.cpp | 4 ++-- lib/Object/RecordStreamer.cpp | 4 ++-- lib/Object/RecordStreamer.h | 3 ++- test/MC/ELF/symbol-binding-changed.s | 6 +++--- tools/llvm-exegesis/lib/SnippetFile.cpp | 3 ++- tools/llvm-mca/CodeRegionGenerator.cpp | 2 +- unittests/CodeGen/TestAsmPrinter.h | 4 ++-- 20 files changed, 47 insertions(+), 38 deletions(-) diff --git a/include/llvm/MC/MCELFStreamer.h b/include/llvm/MC/MCELFStreamer.h index f11629d94e9..0d9735cd3c5 100644 --- a/include/llvm/MC/MCELFStreamer.h +++ b/include/llvm/MC/MCELFStreamer.h @@ -46,7 +46,8 @@ public: void emitAssemblerFlag(MCAssemblerFlag Flag) override; void emitThumbFunc(MCSymbol *Func) override; void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc = SMLoc()) override; void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) override; diff --git a/include/llvm/MC/MCStreamer.h b/include/llvm/MC/MCStreamer.h index 8faa3b0c8ef..68ed85a3615 100644 --- a/include/llvm/MC/MCStreamer.h +++ b/include/llvm/MC/MCStreamer.h @@ -508,8 +508,8 @@ public: virtual void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol); /// Add the given \p Attribute to \p Symbol. - virtual bool emitSymbolAttribute(MCSymbol *Symbol, - MCSymbolAttr Attribute) = 0; + virtual bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc = SMLoc()) = 0; /// Set the \p DescValue for the \p Symbol. /// diff --git a/include/llvm/MC/MCWasmStreamer.h b/include/llvm/MC/MCWasmStreamer.h index 61075e7a573..ded0b3b1676 100644 --- a/include/llvm/MC/MCWasmStreamer.h +++ b/include/llvm/MC/MCWasmStreamer.h @@ -44,7 +44,8 @@ public: void emitAssemblerFlag(MCAssemblerFlag Flag) override; void emitThumbFunc(MCSymbol *Func) override; void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc) override; void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) override; diff --git a/include/llvm/MC/MCWinCOFFStreamer.h b/include/llvm/MC/MCWinCOFFStreamer.h index 53b2ef0bd96..23554756995 100644 --- a/include/llvm/MC/MCWinCOFFStreamer.h +++ b/include/llvm/MC/MCWinCOFFStreamer.h @@ -43,7 +43,8 @@ public: void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override; void emitAssemblerFlag(MCAssemblerFlag Flag) override; void emitThumbFunc(MCSymbol *Func) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc = SMLoc()) override; void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override; void BeginCOFFSymbolDef(MCSymbol const *Symbol) override; void EmitCOFFSymbolStorageClass(int StorageClass) override; diff --git a/include/llvm/MC/MCXCOFFStreamer.h b/include/llvm/MC/MCXCOFFStreamer.h index 5fc2efbe528..a3d5f754b45 100644 --- a/include/llvm/MC/MCXCOFFStreamer.h +++ b/include/llvm/MC/MCXCOFFStreamer.h @@ -19,7 +19,8 @@ public: std::unique_ptr OW, std::unique_ptr Emitter); - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc = SMLoc()) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) override; void emitZerofill(MCSection *Section, MCSymbol *Symbol = nullptr, diff --git a/lib/MC/MCAsmStreamer.cpp b/lib/MC/MCAsmStreamer.cpp index 8d96935b220..7b127c861aa 100644 --- a/lib/MC/MCAsmStreamer.cpp +++ b/lib/MC/MCAsmStreamer.cpp @@ -157,7 +157,8 @@ public: void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override; void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc) override; void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override; void BeginCOFFSymbolDef(const MCSymbol *Symbol) override; @@ -635,7 +636,7 @@ void MCAsmStreamer::emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) { } bool MCAsmStreamer::emitSymbolAttribute(MCSymbol *Symbol, - MCSymbolAttr Attribute) { + MCSymbolAttr Attribute, SMLoc) { switch (Attribute) { case MCSA_Invalid: llvm_unreachable("Invalid symbol attribute"); case MCSA_ELF_TypeFunction: /// .type _foo, STT_FUNC # aka @function diff --git a/lib/MC/MCELFStreamer.cpp b/lib/MC/MCELFStreamer.cpp index 6b653144a8f..4afee0b39f3 100644 --- a/lib/MC/MCELFStreamer.cpp +++ b/lib/MC/MCELFStreamer.cpp @@ -187,7 +187,8 @@ static unsigned CombineSymbolTypes(unsigned T1, unsigned T2) { return T2; } -bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) { +bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute, + SMLoc Loc) { auto *Symbol = cast(S); // Adding a symbol attribute always introduces the symbol, note that an @@ -229,8 +230,8 @@ bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) { // traditionally set the binding to STB_GLOBAL. This is error-prone, so we // error on such cases. Note, we also disallow changed binding from .local. if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_GLOBAL) - getContext().reportError(SMLoc(), Symbol->getName() + - " changed binding to STB_GLOBAL"); + getContext().reportError(Loc, Symbol->getName() + + " changed binding to STB_GLOBAL"); Symbol->setBinding(ELF::STB_GLOBAL); Symbol->setExternal(true); break; @@ -240,16 +241,16 @@ bool MCELFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) { // For `.global x; .weak x`, both MC and GNU as set the binding to STB_WEAK. // We emit a warning for now but may switch to an error in the future. if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_WEAK) - getContext().reportWarning(SMLoc(), Symbol->getName() + - " changed binding to STB_WEAK"); + getContext().reportWarning(Loc, Symbol->getName() + + " changed binding to STB_WEAK"); Symbol->setBinding(ELF::STB_WEAK); Symbol->setExternal(true); break; case MCSA_Local: if (Symbol->isBindingSet() && Symbol->getBinding() != ELF::STB_LOCAL) - getContext().reportError(SMLoc(), Symbol->getName() + - " changed binding to STB_LOCAL"); + getContext().reportError(Loc, Symbol->getName() + + " changed binding to STB_LOCAL"); Symbol->setBinding(ELF::STB_LOCAL); Symbol->setExternal(false); break; diff --git a/lib/MC/MCMachOStreamer.cpp b/lib/MC/MCMachOStreamer.cpp index 2b1d1b28ea1..5312b40c809 100644 --- a/lib/MC/MCMachOStreamer.cpp +++ b/lib/MC/MCMachOStreamer.cpp @@ -93,7 +93,8 @@ public: void emitBuildVersion(unsigned Platform, unsigned Major, unsigned Minor, unsigned Update, VersionTuple SDKVersion) override; void emitThumbFunc(MCSymbol *Func) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc = SMLoc()) override; void emitSymbolDesc(MCSymbol *Symbol, unsigned DescValue) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) override; @@ -290,8 +291,8 @@ void MCMachOStreamer::emitThumbFunc(MCSymbol *Symbol) { cast(Symbol)->setThumbFunc(); } -bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym, - MCSymbolAttr Attribute) { +bool MCMachOStreamer::emitSymbolAttribute(MCSymbol *Sym, MCSymbolAttr Attribute, + SMLoc) { MCSymbolMachO *Symbol = cast(Sym); // Indirect symbols are handled differently, to match how 'as' handles diff --git a/lib/MC/MCNullStreamer.cpp b/lib/MC/MCNullStreamer.cpp index 291d840b4f4..88d550f9a99 100644 --- a/lib/MC/MCNullStreamer.cpp +++ b/lib/MC/MCNullStreamer.cpp @@ -25,8 +25,7 @@ namespace { bool hasRawTextSupport() const override { return true; } void emitRawTextImpl(StringRef String) override {} - bool emitSymbolAttribute(MCSymbol *Symbol, - MCSymbolAttr Attribute) override { + bool emitSymbolAttribute(MCSymbol *, MCSymbolAttr, SMLoc) override { return true; } diff --git a/lib/MC/MCParser/AsmParser.cpp b/lib/MC/MCParser/AsmParser.cpp index f5a06f0a91f..724e6f9891e 100644 --- a/lib/MC/MCParser/AsmParser.cpp +++ b/lib/MC/MCParser/AsmParser.cpp @@ -4881,7 +4881,7 @@ bool AsmParser::parseDirectiveSymbolAttribute(MCSymbolAttr Attr) { if (Sym->isTemporary()) return Error(Loc, "non-local symbol required"); - if (!getStreamer().emitSymbolAttribute(Sym, Attr)) + if (!getStreamer().emitSymbolAttribute(Sym, Attr, Loc)) return Error(Loc, "unable to emit symbol attribute"); return false; }; diff --git a/lib/MC/MCParser/ELFAsmParser.cpp b/lib/MC/MCParser/ELFAsmParser.cpp index 440d3a38191..897743ad028 100644 --- a/lib/MC/MCParser/ELFAsmParser.cpp +++ b/lib/MC/MCParser/ELFAsmParser.cpp @@ -178,13 +178,13 @@ bool ELFAsmParser::ParseDirectiveSymbolAttribute(StringRef Directive, SMLoc) { if (getLexer().isNot(AsmToken::EndOfStatement)) { while (true) { StringRef Name; - + SMLoc Loc = getTok().getLoc(); if (getParser().parseIdentifier(Name)) return TokError("expected identifier in directive"); MCSymbol *Sym = getContext().getOrCreateSymbol(Name); - getStreamer().emitSymbolAttribute(Sym, Attr); + getStreamer().emitSymbolAttribute(Sym, Attr, Loc); if (getLexer().is(AsmToken::EndOfStatement)) break; diff --git a/lib/MC/MCWasmStreamer.cpp b/lib/MC/MCWasmStreamer.cpp index bf8b142b355..531f5f56e53 100644 --- a/lib/MC/MCWasmStreamer.cpp +++ b/lib/MC/MCWasmStreamer.cpp @@ -77,7 +77,8 @@ void MCWasmStreamer::emitWeakReference(MCSymbol *Alias, Alias->setVariableValue(Value); } -bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute) { +bool MCWasmStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute, + SMLoc) { assert(Attribute != MCSA_IndirectSymbol && "indirect symbols not supported"); auto *Symbol = cast(S); diff --git a/lib/MC/MCWinCOFFStreamer.cpp b/lib/MC/MCWinCOFFStreamer.cpp index 97cceac74ac..c671dc0d173 100644 --- a/lib/MC/MCWinCOFFStreamer.cpp +++ b/lib/MC/MCWinCOFFStreamer.cpp @@ -107,8 +107,8 @@ void MCWinCOFFStreamer::emitThumbFunc(MCSymbol *Func) { llvm_unreachable("not implemented"); } -bool MCWinCOFFStreamer::emitSymbolAttribute(MCSymbol *S, - MCSymbolAttr Attribute) { +bool MCWinCOFFStreamer::emitSymbolAttribute(MCSymbol *S, MCSymbolAttr Attribute, + SMLoc) { auto *Symbol = cast(S); getAssembler().registerSymbol(*Symbol); diff --git a/lib/MC/MCXCOFFStreamer.cpp b/lib/MC/MCXCOFFStreamer.cpp index ec9e89fac41..2b4ffaed3dd 100644 --- a/lib/MC/MCXCOFFStreamer.cpp +++ b/lib/MC/MCXCOFFStreamer.cpp @@ -29,8 +29,8 @@ MCXCOFFStreamer::MCXCOFFStreamer(MCContext &Context, : MCObjectStreamer(Context, std::move(MAB), std::move(OW), std::move(Emitter)) {} -bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym, - MCSymbolAttr Attribute) { +bool MCXCOFFStreamer::emitSymbolAttribute(MCSymbol *Sym, MCSymbolAttr Attribute, + SMLoc) { auto *Symbol = cast(Sym); getAssembler().registerSymbol(*Symbol); diff --git a/lib/Object/RecordStreamer.cpp b/lib/Object/RecordStreamer.cpp index b2f973eff36..282b1c63a8f 100644 --- a/lib/Object/RecordStreamer.cpp +++ b/lib/Object/RecordStreamer.cpp @@ -97,7 +97,7 @@ void RecordStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) { } bool RecordStreamer::emitSymbolAttribute(MCSymbol *Symbol, - MCSymbolAttr Attribute) { + MCSymbolAttr Attribute, SMLoc) { if (Attribute == MCSA_Global || Attribute == MCSA_Weak) markGlobal(*Symbol, Attribute); if (Attribute == MCSA_LazyReference) @@ -226,7 +226,7 @@ void RecordStreamer::flushSymverDirectives() { // Don't use EmitAssignment override as it always marks alias as defined. MCStreamer::emitAssignment(Alias, Value); if (Attr != MCSA_Invalid) - emitSymbolAttribute(Alias, Attr); + emitSymbolAttribute(Alias, Attr, SMLoc()); } } } diff --git a/lib/Object/RecordStreamer.h b/lib/Object/RecordStreamer.h index 99d15f790a1..fb1122e8410 100644 --- a/lib/Object/RecordStreamer.h +++ b/lib/Object/RecordStreamer.h @@ -48,7 +48,8 @@ public: void emitInstruction(const MCInst &Inst, const MCSubtargetInfo &STI) override; void emitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override; void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override; - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override; + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc Loc) override; void emitZerofill(MCSection *Section, MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment, SMLoc Loc = SMLoc()) override; void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, diff --git a/test/MC/ELF/symbol-binding-changed.s b/test/MC/ELF/symbol-binding-changed.s index f327fae2a1c..33be9062958 100644 --- a/test/MC/ELF/symbol-binding-changed.s +++ b/test/MC/ELF/symbol-binding-changed.s @@ -1,17 +1,17 @@ # RUN: not llvm-mc -filetype=obj -triple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error: -# CHECK: error: local changed binding to STB_GLOBAL +# CHECK: {{.*}}.s:[[#@LINE+3]]:8: error: local changed binding to STB_GLOBAL local: .local local .globl local ## `.globl x; .weak x` matches the GNU as behavior. We issue a warning for now. -# CHECK: warning: global changed binding to STB_WEAK +# CHECK: {{.*}}.s:[[#@LINE+3]]:7: warning: global changed binding to STB_WEAK global: .global global .weak global -# CHECK: error: weak changed binding to STB_LOCAL +# CHECK: {{.*}}.s:[[#@LINE+3]]:8: error: weak changed binding to STB_LOCAL weak: .weak weak .local weak diff --git a/tools/llvm-exegesis/lib/SnippetFile.cpp b/tools/llvm-exegesis/lib/SnippetFile.cpp index c71050c61fa..63be04f307c 100644 --- a/tools/llvm-exegesis/lib/SnippetFile.cpp +++ b/tools/llvm-exegesis/lib/SnippetFile.cpp @@ -89,7 +89,8 @@ private: // We only care about instructions, we don't implement this part of the API. void emitCommonSymbol(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment) override {} - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override { + bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute, + SMLoc) override { return false; } void emitValueToAlignment(unsigned ByteAlignment, int64_t Value, diff --git a/tools/llvm-mca/CodeRegionGenerator.cpp b/tools/llvm-mca/CodeRegionGenerator.cpp index 831b76ab80c..df3dbd9184d 100644 --- a/tools/llvm-mca/CodeRegionGenerator.cpp +++ b/tools/llvm-mca/CodeRegionGenerator.cpp @@ -52,7 +52,7 @@ public: Regions.addInstruction(Inst); } - bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override { + bool emitSymbolAttribute(MCSymbol *, MCSymbolAttr, SMLoc) override { return true; } diff --git a/unittests/CodeGen/TestAsmPrinter.h b/unittests/CodeGen/TestAsmPrinter.h index 65e557b9b4a..bfc67f5c689 100644 --- a/unittests/CodeGen/TestAsmPrinter.h +++ b/unittests/CodeGen/TestAsmPrinter.h @@ -28,8 +28,8 @@ public: // These methods are pure virtual in MCStreamer, thus, have to be overridden: - MOCK_METHOD2(emitSymbolAttribute, - bool(MCSymbol *Symbol, MCSymbolAttr Attribute)); + MOCK_METHOD3(emitSymbolAttribute, + bool(MCSymbol *Symbol, MCSymbolAttr Attribute, SMLoc Loc)); MOCK_METHOD3(emitCommonSymbol, void(MCSymbol *Symbol, uint64_t Size, unsigned ByteAlignment)); MOCK_METHOD5(emitZerofill,