[llvm] r233595 - MC: For variable symbols, maintain MCSymbol::Section as a cache.
Jim Grosbach
grosbach at apple.com
Wed Apr 1 13:15:36 PDT 2015
Uh….. I had not signed off on this change and had already objected to the completely insufficient commit message, which you have not corrected. Why?
> On Mar 30, 2015, at 1:41 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
>
> Author: pcc
> Date: Mon Mar 30 15:41:21 2015
> New Revision: 233595
>
> URL: http://llvm.org/viewvc/llvm-project?rev=233595&view=rev
> Log:
> MC: For variable symbols, maintain MCSymbol::Section as a cache.
>
> This fixes the visibility of symbols in certain edge cases involving aliases
> with multiple levels of indirection.
>
> Fixes PR19582.
>
> Differential Revision: http://reviews.llvm.org/D8586
>
> Added:
> llvm/trunk/test/MC/ELF/alias-resolve.s
> Modified:
> llvm/trunk/include/llvm/MC/MCMachObjectWriter.h
> llvm/trunk/include/llvm/MC/MCSymbol.h
> llvm/trunk/lib/MC/MCExpr.cpp
> llvm/trunk/lib/MC/MCSymbol.cpp
> llvm/trunk/lib/MC/MachObjectWriter.cpp
> llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp
> llvm/trunk/test/MC/ELF/alias.s
> llvm/trunk/test/MC/MachO/ARM/aliased-symbols.s
>
> Modified: llvm/trunk/include/llvm/MC/MCMachObjectWriter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCMachObjectWriter.h?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCMachObjectWriter.h (original)
> +++ llvm/trunk/include/llvm/MC/MCMachObjectWriter.h Mon Mar 30 15:41:21 2015
> @@ -257,8 +257,6 @@ public:
> void computeSectionAddresses(const MCAssembler &Asm,
> const MCAsmLayout &Layout);
>
> - void markAbsoluteVariableSymbols(MCAssembler &Asm,
> - const MCAsmLayout &Layout);
> void ExecutePostLayoutBinding(MCAssembler &Asm,
> const MCAsmLayout &Layout) override;
>
>
> Modified: llvm/trunk/include/llvm/MC/MCSymbol.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSymbol.h?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCSymbol.h (original)
> +++ llvm/trunk/include/llvm/MC/MCSymbol.h Mon Mar 30 15:41:21 2015
> @@ -15,6 +15,7 @@
> #define LLVM_MC_MCSYMBOL_H
>
> #include "llvm/ADT/StringRef.h"
> +#include "llvm/MC/MCExpr.h"
> #include "llvm/Support/Compiler.h"
>
> namespace llvm {
> @@ -42,8 +43,9 @@ namespace llvm {
>
> /// Section - The section the symbol is defined in. This is null for
> /// undefined symbols, and the special AbsolutePseudoSection value for
> - /// absolute symbols.
> - const MCSection *Section;
> + /// absolute symbols. If this is a variable symbol, this caches the
> + /// variable value's section.
> + mutable const MCSection *Section;
>
> /// Value - If non-null, the value for a variable symbol.
> const MCExpr *Value;
> @@ -68,6 +70,12 @@ namespace llvm {
>
> MCSymbol(const MCSymbol&) = delete;
> void operator=(const MCSymbol&) = delete;
> + const MCSection *getSectionPtr() const {
> + if (Section || !Value)
> + return Section;
> + return Section = Value->FindAssociatedSection();
> + }
> +
> public:
> /// getName - Get the symbol name.
> StringRef getName() const { return Name; }
> @@ -103,7 +111,7 @@ namespace llvm {
> ///
> /// Defined symbols are either absolute or in some section.
> bool isDefined() const {
> - return Section != nullptr;
> + return getSectionPtr() != nullptr;
> }
>
> /// isInSection - Check if this symbol is defined in some section (i.e., it
> @@ -119,27 +127,27 @@ namespace llvm {
>
> /// isAbsolute - Check if this is an absolute symbol.
> bool isAbsolute() const {
> - return Section == AbsolutePseudoSection;
> + return getSectionPtr() == AbsolutePseudoSection;
> }
>
> /// getSection - Get the section associated with a defined, non-absolute
> /// symbol.
> const MCSection &getSection() const {
> assert(isInSection() && "Invalid accessor!");
> - return *Section;
> + return *getSectionPtr();
> }
>
> /// setSection - Mark the symbol as defined in the section \p S.
> - void setSection(const MCSection &S) { Section = &S; }
> + void setSection(const MCSection &S) {
> + assert(!isVariable() && "Cannot set section of variable");
> + Section = &S;
> + }
>
> /// setUndefined - Mark the symbol as undefined.
> void setUndefined() {
> Section = nullptr;
> }
>
> - /// setAbsolute - Mark the symbol as absolute.
> - void setAbsolute() { Section = AbsolutePseudoSection; }
> -
> /// @}
> /// @name Variable Symbols
> /// @{
>
> Modified: llvm/trunk/lib/MC/MCExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCExpr.cpp?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCExpr.cpp (original)
> +++ llvm/trunk/lib/MC/MCExpr.cpp Mon Mar 30 15:41:21 2015
> @@ -775,6 +775,9 @@ const MCSection *MCExpr::FindAssociatedS
> if (RHS_S == MCSymbol::AbsolutePseudoSection)
> return LHS_S;
>
> + if (BE->getOpcode() == MCBinaryExpr::Sub)
> + return MCSymbol::AbsolutePseudoSection;
> +
> // Otherwise, return the first non-null section.
> return LHS_S ? LHS_S : RHS_S;
> }
>
> Modified: llvm/trunk/lib/MC/MCSymbol.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSymbol.cpp?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCSymbol.cpp (original)
> +++ llvm/trunk/lib/MC/MCSymbol.cpp Mon Mar 30 15:41:21 2015
> @@ -55,13 +55,7 @@ void MCSymbol::setVariableValue(const MC
> assert(!IsUsed && "Cannot set a variable that has already been used.");
> assert(Value && "Invalid variable value!");
> this->Value = Value;
> -
> - // Variables should always be marked as in the same "section" as the value.
> - const MCSection *Section = Value->FindAssociatedSection();
> - if (Section)
> - setSection(*Section);
> - else
> - setUndefined();
> + this->Section = nullptr;
> }
>
> void MCSymbol::print(raw_ostream &OS) const {
>
> Modified: llvm/trunk/lib/MC/MachObjectWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MachObjectWriter.cpp?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MachObjectWriter.cpp (original)
> +++ llvm/trunk/lib/MC/MachObjectWriter.cpp Mon Mar 30 15:41:21 2015
> @@ -649,33 +649,12 @@ void MachObjectWriter::computeSectionAdd
> }
> }
>
> -void MachObjectWriter::markAbsoluteVariableSymbols(MCAssembler &Asm,
> - const MCAsmLayout &Layout) {
> - for (MCSymbolData &SD : Asm.symbols()) {
> - if (!SD.getSymbol().isVariable())
> - continue;
> -
> - // Is the variable is a symbol difference (SA - SB + C) expression,
> - // and neither symbol is external, mark the variable as absolute.
> - const MCExpr *Expr = SD.getSymbol().getVariableValue();
> - MCValue Value;
> - if (Expr->EvaluateAsRelocatable(Value, &Layout, nullptr)) {
> - if (Value.getSymA() && Value.getSymB())
> - const_cast<MCSymbol*>(&SD.getSymbol())->setAbsolute();
> - }
> - }
> -}
> -
> void MachObjectWriter::ExecutePostLayoutBinding(MCAssembler &Asm,
> const MCAsmLayout &Layout) {
> computeSectionAddresses(Asm, Layout);
>
> // Create symbol data for any indirect symbols.
> BindIndirectSymbols(Asm);
> -
> - // Mark symbol difference expressions in variables (from .set or = directives)
> - // as absolute.
> - markAbsoluteVariableSymbols(Asm, Layout);
> }
>
> bool MachObjectWriter::
>
> Modified: llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86AsmPrinter.cpp Mon Mar 30 15:41:21 2015
> @@ -523,7 +523,6 @@ void X86AsmPrinter::EmitStartOfAsmFile(M
> // must be registered in .sxdata. Use of any unregistered handlers will
> // cause the process to terminate immediately. LLVM does not know how to
> // register any SEH handlers, so its object files should be safe.
> - S->setAbsolute();
> OutStreamer.EmitSymbolAttribute(S, MCSA_Global);
> OutStreamer.EmitAssignment(
> S, MCConstantExpr::Create(int64_t(1), MMI->getContext()));
>
> Added: llvm/trunk/test/MC/ELF/alias-resolve.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/alias-resolve.s?rev=233595&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/alias-resolve.s (added)
> +++ llvm/trunk/test/MC/ELF/alias-resolve.s Mon Mar 30 15:41:21 2015
> @@ -0,0 +1,8 @@
> +// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -r | FileCheck %s
> +
> +a:
> + .section foo
> + c = b
> +b:
> + // CHECK: 0x0 R_X86_64_PC32 .text 0x0
> + .long a - c
>
> Modified: llvm/trunk/test/MC/ELF/alias.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/alias.s?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/alias.s (original)
> +++ llvm/trunk/test/MC/ELF/alias.s Mon Mar 30 15:41:21 2015
> @@ -24,6 +24,15 @@ bar5 = bar4
> bar6 = bar5
> bar6:
>
> +// Test that indirect local aliases do not appear as symbols.
> +.data
> +.Llocal:
> +
> +.text
> +leaq .Llocal1(%rip), %rdi
> +.Llocal1 = .Llocal2
> +.Llocal2 = .Llocal
> +
> // CHECK: Symbols [
> // CHECK-NEXT: Symbol {
> // CHECK-NEXT: Name: (0)
>
> Modified: llvm/trunk/test/MC/MachO/ARM/aliased-symbols.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/MachO/ARM/aliased-symbols.s?rev=233595&r1=233594&r2=233595&view=diff
> ==============================================================================
> --- llvm/trunk/test/MC/MachO/ARM/aliased-symbols.s (original)
> +++ llvm/trunk/test/MC/MachO/ARM/aliased-symbols.s Mon Mar 30 15:41:21 2015
> @@ -45,9 +45,9 @@ Ltmp0:
> // CHECK-NEXT: Value: 0x[[DEFINED_EARLY]]
> // CHECK-NEXT: }
>
> - // defined_late was defined. Just after defined_early.
> + // alias_to_late was an alias to defined_late. But we can resolve it.
> // CHECK: Symbol {
> -// CHECK-NEXT: Name: defined_late
> +// CHECK-NEXT: Name: alias_to_late
> // CHECK-NEXT: Type: Section (0xE)
> // CHECK-NEXT: Section: __data (0x2)
> // CHECK-NEXT: RefType: UndefinedNonLazy (0x0)
> @@ -56,9 +56,9 @@ Ltmp0:
> // CHECK-NEXT: Value: 0x[[DEFINED_LATE:[0-9A-F]+]]
> // CHECK-NEXT: }
>
> - // alias_to_late was an alias to defined_late. But we can resolve it.
> + // defined_late was defined. Just after defined_early.
> // CHECK: Symbol {
> -// CHECK-NEXT: Name: alias_to_late
> +// CHECK-NEXT: Name: defined_late
> // CHECK-NEXT: Type: Section (0xE)
> // CHECK-NEXT: Section: __data (0x2)
> // CHECK-NEXT: RefType: UndefinedNonLazy (0x0)
> @@ -72,7 +72,7 @@ Ltmp0:
> // CHECK: Symbol {
> // CHECK-NEXT: Name: alias_to_local (42)
> // CHECK-NEXT: Type: Section (0xE)
> -// CHECK-NEXT: Section: (0x0)
> +// CHECK-NEXT: Section: __data (0x2)
> // CHECK-NEXT: RefType: UndefinedNonLazy (0x0)
> // CHECK-NEXT: Flags [ (0x0)
> // CHECK-NEXT: ]
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list