[llvm] r203962 - Correctly handle an ELF symbol defined with "a = b + expr".
Alexander Kornienko
alexfh at google.com
Mon Mar 17 16:43:43 PDT 2014
Here's a small repro (clang-3.5-bad is current HEAD, clang-3.5-good is
built after svn up -r203961 lib/MC/MCSymbol.cpp include/llvm/MC/MCSymbol.h
lib/MC/ELFObjectWriter.cpp):
$ cat q.c
void a1();
void a2() __attribute__((alias("a1")));
void a3() __attribute__((alias("a2")));
void a1() {}
$ ~/work/llvm-build/bin/clang-3.5-good -c q.c && mv q.o good.o &&
~/work/llvm-build/bin/clang-3.5-bad -c q.c && mv q.o bad.o && objdump -t
good.o bad.o
good.o: file format elf64-x86-64
SYMBOL TABLE:
0000000000000000 l df *ABS* 0000000000000000 q.c
0000000000000000 l d .text 0000000000000000 .text
0000000000000000 l d .data 0000000000000000 .data
0000000000000000 l d .bss 0000000000000000 .bss
0000000000000000 l d .comment 0000000000000000 .comment
0000000000000000 l d .note.GNU-stack 0000000000000000
.note.GNU-stack
0000000000000000 l d .eh_frame 0000000000000000 .eh_frame
0000000000000000 g F .text 0000000000000006 a1
0000000000000000 g F .text 0000000000000006 a2
0000000000000000 g F .text 0000000000000006 a3
bad.o: file format elf64-x86-64
SYMBOL TABLE:
0000000000000000 l df *ABS* 0000000000000000 q.c
0000000000000000 l d .text 0000000000000000 .text
0000000000000000 l d .data 0000000000000000 .data
0000000000000000 l d .bss 0000000000000000 .bss
0000000000000000 l d .comment 0000000000000000 .comment
0000000000000000 l d .note.GNU-stack 0000000000000000
.note.GNU-stack
0000000000000000 l d .eh_frame 0000000000000000 .eh_frame
0000000000000000 g F .text 0000000000000006 a1
0000000000000000 g F .text 0000000000000006 a2
0000000000000000 g .text 0000000000000000 a3
Difference is in the flags of the symbol a3.
Could you please fix the problem or revert the commit?
Thanks!
On Mon, Mar 17, 2014 at 8:05 PM, Alexander Kornienko <alexfh at google.com>wrote:
> 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.
>>>
>>> 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/20140318/dfdd50b8/attachment.html>
More information about the llvm-commits
mailing list