# [llvm] r203962 - Correctly handle an ELF symbol defined with "a = b + expr".

Mon Mar 17 12:05:52 PDT 2014

```On Mon, Mar 17, 2014 at 5:17 PM, Alexander Kornienko <alexfh at google.com>wrote:

> 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.
>
> +David, who may be interested in this.

I've found a translation unit, which is handled differently before and
after this revision. The difference is that after this revision, the
functions marked with __attribute__((alias("..."))) loose the "function"
flag (i.e. they get space instead of 'F' in the third column of the objdump
-t output). Is this enough or you need more information to get this fixed?

>
>
> 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.
>>
>>     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!");
>>
>> 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 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/94af0f0f/attachment.html>
```