[llvm] r203962 - Correctly handle an ELF symbol defined with "a = b + expr".
Alexander Kornienko
alexfh at google.com
Mon Mar 17 09:17:24 PDT 2014
This commit breaks something in our code base. Unfortunately, I don't know
what exactly and I'm struggling to come up with a reasonably-sized
reproduction. Once I have something specific, I'll let you know.
On Fri, Mar 14, 2014 at 9:09 PM, Rafael Espindola <
rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Fri Mar 14 15:09:04 2014
> New Revision: 203962
>
> URL: http://llvm.org/viewvc/llvm-project?rev=203962&view=rev
> Log:
> Correctly handle an ELF symbol defined with "a = b + expr".
>
> We were marking the symbol as absolute instead of computing b's offset +
> the
> expression value.
>
> This fixes pr19126.
>
> Added:
> llvm/trunk/test/MC/ELF/offset.s
> Modified:
> llvm/trunk/include/llvm/MC/MCSymbol.h
> llvm/trunk/lib/MC/ELFObjectWriter.cpp
> llvm/trunk/lib/MC/MCSymbol.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCSymbol.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSymbol.h?rev=203962&r1=203961&r2=203962&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCSymbol.h (original)
> +++ llvm/trunk/include/llvm/MC/MCSymbol.h Fri Mar 14 15:09:04 2014
> @@ -18,6 +18,7 @@
> #include "llvm/Support/Compiler.h"
>
> namespace llvm {
> + class MCAsmLayout;
> class MCExpr;
> class MCSection;
> class MCContext;
> @@ -145,6 +146,11 @@ namespace llvm {
> // itself.
> const MCSymbol &AliasedSymbol() const;
>
> + // If this symbol is not a variable, return itself. If it is a
> variable,
> + // evaluate it and check if it is of the form Base + ConstantOffset.
> If so,
> + // return Base, if not, return nullptr.
> + const MCSymbol *getBaseSymbol(const MCAsmLayout &Layout) const;
> +
> void setVariableValue(const MCExpr *Value);
>
> /// @}
>
> Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=203962&r1=203961&r2=203962&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
> +++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Fri Mar 14 15:09:04 2014
> @@ -271,6 +271,7 @@ class ELFObjectWriter : public MCObjectW
> /// \param RevGroupMap - Maps a signature symbol to the group section.
> /// \param NumRegularSections - Number of non-relocation sections.
> void ComputeSymbolTable(MCAssembler &Asm,
> + const MCAsmLayout &Layout,
> const SectionIndexMapTy &SectionIndexMap,
> RevGroupMapTy RevGroupMap,
> unsigned NumRegularSections);
> @@ -462,33 +463,43 @@ void ELFObjectWriter::WriteSymbolEntry(M
> }
> }
>
> -uint64_t ELFObjectWriter::SymbolValue(MCSymbolData &Data,
> +uint64_t ELFObjectWriter::SymbolValue(MCSymbolData &OrigData,
> const MCAsmLayout &Layout) {
> - if (Data.isCommon() && Data.isExternal())
> - return Data.getCommonAlignment();
> -
> - const MCSymbol &Symbol = Data.getSymbol();
> + MCSymbolData *Data = &OrigData;
> + if (Data->isCommon() && Data->isExternal())
> + return Data->getCommonAlignment();
> +
> + const MCSymbol *Symbol = &Data->getSymbol();
> +
> + uint64_t Res = 0;
> + if (Symbol->isVariable()) {
> + const MCExpr *Expr = Symbol->getVariableValue();
> + MCValue Value;
> + if (!Expr->EvaluateAsRelocatable(Value, &Layout))
> + return 0;
> + if (Value.getSymB())
> + return 0;
> + Res = Value.getConstant();
> +
> + const MCSymbolRefExpr *A = Value.getSymA();
> + if (!A)
> + return Res;
>
> - if (Symbol.isAbsolute() && Symbol.isVariable()) {
> - if (const MCExpr *Value = Symbol.getVariableValue()) {
> - int64_t IntValue;
> - if (Value->EvaluateAsAbsolute(IntValue, Layout))
> - return (uint64_t)IntValue;
> - }
> + Symbol = &A->getSymbol();
> + Data = &Layout.getAssembler().getSymbolData(*Symbol);
> }
>
> - if (!Symbol.isInSection())
> + if (!Symbol->isInSection())
> return 0;
>
> + if (!Data->getFragment())
> + return 0;
>
> - if (Data.getFragment()) {
> - if (Data.getFlags() & ELF_Other_ThumbFunc)
> - return Layout.getSymbolOffset(&Data)+1;
> - else
> - return Layout.getSymbolOffset(&Data);
> - }
> + Res += Layout.getSymbolOffset(Data);
> + if (Data->getFlags() & ELF_Other_ThumbFunc)
> + ++Res;
>
> - return 0;
> + return Res;
> }
>
> void ELFObjectWriter::ExecutePostLayoutBinding(MCAssembler &Asm,
> @@ -586,7 +597,7 @@ void ELFObjectWriter::WriteSymbol(MCData
> uint8_t Other = MCELF::getOther(OrigData) << (ELF_STO_Shift -
> ELF_STV_Shift);
> Other |= Visibility;
>
> - uint64_t Value = SymbolValue(Data, Layout);
> + uint64_t Value = SymbolValue(OrigData, Layout);
> uint64_t Size = 0;
>
> assert(!(Data.isCommon() && !Data.isExternal()));
> @@ -897,10 +908,11 @@ void ELFObjectWriter::ComputeIndexMap(MC
> }
> }
>
> -void ELFObjectWriter::ComputeSymbolTable(MCAssembler &Asm,
> - const SectionIndexMapTy
> &SectionIndexMap,
> - RevGroupMapTy RevGroupMap,
> - unsigned NumRegularSections) {
> +void
> +ELFObjectWriter::ComputeSymbolTable(MCAssembler &Asm, const MCAsmLayout
> &Layout,
> + const SectionIndexMapTy
> &SectionIndexMap,
> + RevGroupMapTy RevGroupMap,
> + unsigned NumRegularSections) {
> // FIXME: Is this the correct place to do this?
> // FIXME: Why is an undefined reference to _GLOBAL_OFFSET_TABLE_ needed?
> if (NeedsGOT) {
> @@ -948,33 +960,33 @@ void ELFObjectWriter::ComputeSymbolTable
>
> ELFSymbolData MSD;
> MSD.SymbolData = it;
> - const MCSymbol &RefSymbol = Symbol.AliasedSymbol();
> + const MCSymbol *BaseSymbol = Symbol.getBaseSymbol(Layout);
>
> // Undefined symbols are global, but this is the first place we
> // are able to set it.
> bool Local = isLocal(*it, isSignature, Used);
> if (!Local && MCELF::GetBinding(*it) == ELF::STB_LOCAL) {
> - MCSymbolData &SD = Asm.getSymbolData(RefSymbol);
> + assert(BaseSymbol);
> + MCSymbolData &SD = Asm.getSymbolData(*BaseSymbol);
> MCELF::SetBinding(*it, ELF::STB_GLOBAL);
> MCELF::SetBinding(SD, ELF::STB_GLOBAL);
> }
>
> - if (RefSymbol.isUndefined() && !Used && WeakrefUsed)
> - MCELF::SetBinding(*it, ELF::STB_WEAK);
> -
> - if (it->isCommon()) {
> + if (!BaseSymbol) {
> + MSD.SectionIndex = ELF::SHN_ABS;
> + } else if (it->isCommon()) {
> assert(!Local);
> MSD.SectionIndex = ELF::SHN_COMMON;
> - } else if (Symbol.isAbsolute() || RefSymbol.isVariable()) {
> - MSD.SectionIndex = ELF::SHN_ABS;
> - } else if (RefSymbol.isUndefined()) {
> + } else if (BaseSymbol->isUndefined()) {
> if (isSignature && !Used)
> MSD.SectionIndex = SectionIndexMap.lookup(RevGroupMap[&Symbol]);
> else
> MSD.SectionIndex = ELF::SHN_UNDEF;
> + if (!Used && WeakrefUsed)
> + MCELF::SetBinding(*it, ELF::STB_WEAK);
> } else {
> const MCSectionELF &Section =
> - static_cast<const MCSectionELF&>(RefSymbol.getSection());
> + static_cast<const MCSectionELF&>(BaseSymbol->getSection());
> MSD.SectionIndex = SectionIndexMap.lookup(&Section);
> if (MSD.SectionIndex >= ELF::SHN_LORESERVE)
> NeedsSymtabShndx = true;
> @@ -1560,8 +1572,8 @@ void ELFObjectWriter::WriteObject(MCAsse
> unsigned NumRegularSections = NumUserSections + NumIndexedSections;
>
> // Compute symbol table information.
> - ComputeSymbolTable(Asm, SectionIndexMap, RevGroupMap,
> NumRegularSections);
> -
> + ComputeSymbolTable(Asm, Layout, SectionIndexMap, RevGroupMap,
> + NumRegularSections);
>
> WriteRelocations(Asm, const_cast<MCAsmLayout&>(Layout), RelMap);
>
>
> Modified: llvm/trunk/lib/MC/MCSymbol.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSymbol.cpp?rev=203962&r1=203961&r2=203962&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCSymbol.cpp (original)
> +++ llvm/trunk/lib/MC/MCSymbol.cpp Fri Mar 14 15:09:04 2014
> @@ -9,6 +9,7 @@
>
> #include "llvm/MC/MCSymbol.h"
> #include "llvm/MC/MCExpr.h"
> +#include "llvm/MC/MCValue.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/raw_ostream.h"
> using namespace llvm;
> @@ -51,6 +52,22 @@ const MCSymbol &MCSymbol::AliasedSymbol(
> return *S;
> }
>
> +const MCSymbol *MCSymbol::getBaseSymbol(const MCAsmLayout &Layout) const {
> + if (!isVariable())
> + return this;
> +
> + const MCExpr *Expr = getVariableValue();
> + MCValue Value;
> + if (!Expr->EvaluateAsRelocatable(Value, &Layout))
> + return nullptr;
> +
> + if (Value.getSymB())
> + return nullptr;
> + if (const MCSymbolRefExpr *A = Value.getSymA())
> + return &A->getSymbol();
> + return nullptr;
> +}
> +
> void MCSymbol::setVariableValue(const MCExpr *Value) {
> assert(!IsUsed && "Cannot set a variable that has already been used.");
> assert(Value && "Invalid variable value!");
>
> Added: llvm/trunk/test/MC/ELF/offset.s
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/offset.s?rev=203962&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/offset.s (added)
> +++ llvm/trunk/test/MC/ELF/offset.s Fri Mar 14 15:09:04 2014
> @@ -0,0 +1,58 @@
> +// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - |
> llvm-readobj -t - | FileCheck %s
> +
> +// Test that a variable declared with "var = other_var + cst" is in the
> same
> +// section as other_var and its value is the value of other_var + cst.
> +
> + .data
> + .globl sym_a
> + .byte 42
> +sym_a:
> +
> +// CHECK: Symbol {
> +// CHECK: Name: sym_a
> +// CHECK-NEXT: Value: 0x1
> +// CHECK-NEXT: Size: 0
> +// CHECK-NEXT: Binding: Global
> +// CHECK-NEXT: Type: None
> +// CHECK-NEXT: Other: 0
> +// CHECK-NEXT: Section: .data
> +// CHECK-NEXT: }
> +
> + .long 42
> + .globl sym_b
> +sym_b:
> + .globl sym_c
> +sym_c = sym_a
> +// CHECK: Symbol {
> +// CHECK: Name: sym_c
> +// CHECK-NEXT: Value: 0x1
> +// CHECK-NEXT: Size: 0
> +// CHECK-NEXT: Binding: Global
> +// CHECK-NEXT: Type: None
> +// CHECK-NEXT: Other: 0
> +// CHECK-NEXT: Section: .data
> +// CHECK-NEXT: }
> +
> + .globl sym_d
> +sym_d = sym_a + 1
> +// CHECK: Symbol {
> +// CHECK: Name: sym_d
> +// CHECK-NEXT: Value: 0x2
> +// CHECK-NEXT: Size: 0
> +// CHECK-NEXT: Binding: Global
> +// CHECK-NEXT: Type: None
> +// CHECK-NEXT: Other: 0
> +// CHECK-NEXT: Section: .data
> +// CHECK-NEXT: }
> +
> + .globl sym_e
> +sym_e = sym_a + (sym_b - sym_a) * 3
> +// CHECK: Symbol {
> +// CHECK: Name: sym_e
> +// CHECK-NEXT: Value: 0xD
> +// CHECK-NEXT: Size: 0
> +// CHECK-NEXT: Binding: Global
> +// CHECK-NEXT: Type: None
> +// CHECK-NEXT: Other: 0
> +// CHECK-NEXT: Section: .data
> +// CHECK-NEXT: }
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140317/daa57e63/attachment.html>
More information about the llvm-commits
mailing list