[llvm] r193569 - Move the STT_FILE symbols out of the normal symbol table processing for

Rafael EspĂ­ndola rafael.espindola at gmail.com
Mon Oct 28 18:38:57 PDT 2013


thanks. This was one of the most amusing bugs we had!

On 28 October 2013 21:06, Joerg Sonnenberger <joerg at bec.de> wrote:
> Author: joerg
> Date: Mon Oct 28 20:06:17 2013
> New Revision: 193569
>
> URL: http://llvm.org/viewvc/llvm-project?rev=193569&view=rev
> Log:
> Move the STT_FILE symbols out of the normal symbol table processing for
> ELF. They can overlap with the other symbols, e.g. if a source file
> "foo.c" contains a function "foo" with a static variable "c".
>
> Added:
>     llvm/trunk/test/MC/ELF/file-double.s
> Modified:
>     llvm/trunk/include/llvm/MC/MCAssembler.h
>     llvm/trunk/lib/MC/ELFObjectWriter.cpp
>     llvm/trunk/lib/MC/MCELF.cpp
>     llvm/trunk/lib/MC/MCELFStreamer.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCAssembler.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAssembler.h?rev=193569&r1=193568&r2=193569&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAssembler.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAssembler.h Mon Oct 28 20:06:17 2013
> @@ -19,6 +19,7 @@
>  #include "llvm/MC/MCInst.h"
>  #include "llvm/Support/Casting.h"
>  #include "llvm/Support/DataTypes.h"
> +#include <algorithm>
>  #include <vector> // FIXME: Shouldn't be needed.
>
>  namespace llvm {
> @@ -816,6 +817,9 @@ public:
>    typedef SymbolDataListType::const_iterator const_symbol_iterator;
>    typedef SymbolDataListType::iterator symbol_iterator;
>
> +  typedef std::vector<std::string> FileNameVectorType;
> +  typedef FileNameVectorType::const_iterator const_file_name_iterator;
> +
>    typedef std::vector<IndirectSymbolData>::const_iterator
>      const_indirect_symbol_iterator;
>    typedef std::vector<IndirectSymbolData>::iterator indirect_symbol_iterator;
> @@ -859,6 +863,9 @@ private:
>    /// The list of linker options to propagate into the object file.
>    std::vector<std::vector<std::string> > LinkerOptions;
>
> +  /// List of declared file names
> +  FileNameVectorType FileNames;
> +
>    /// The set of function symbols for which a .thumb_func directive has
>    /// been seen.
>    //
> @@ -1152,6 +1159,20 @@ public:
>      return *Entry;
>    }
>
> +  const_file_name_iterator file_names_begin() const {
> +    return FileNames.begin();
> +  }
> +
> +  const_file_name_iterator file_names_end() const {
> +    return FileNames.end();
> +  }
> +
> +  void addFileName(StringRef FileName) {
> +    if (std::find(file_names_begin(), file_names_end(), FileName) ==
> +        file_names_end())
> +      FileNames.push_back(FileName);
> +  }
> +
>    /// @}
>
>    void dump();
>
> Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=193569&r1=193568&r2=193569&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
> +++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Mon Oct 28 20:06:17 2013
> @@ -73,10 +73,6 @@ class ELFObjectWriter : public MCObjectW
>
>        // Support lexicographic sorting.
>        bool operator<(const ELFSymbolData &RHS) const {
> -        if (MCELF::GetType(*SymbolData) == ELF::STT_FILE)
> -          return true;
> -        if (MCELF::GetType(*RHS.SymbolData) == ELF::STT_FILE)
> -          return false;
>          return SymbolData->getSymbol().getName() <
>                 RHS.SymbolData->getSymbol().getName();
>        }
> @@ -98,6 +94,7 @@ class ELFObjectWriter : public MCObjectW
>      /// @{
>
>      SmallString<256> StringTable;
> +    std::vector<uint64_t> FileSymbolData;
>      std::vector<ELFSymbolData> LocalSymbolData;
>      std::vector<ELFSymbolData> ExternalSymbolData;
>      std::vector<ELFSymbolData> UndefinedSymbolData;
> @@ -551,7 +548,7 @@ void ELFObjectWriter::WriteSymbol(MCData
>    uint8_t Type = MCELF::GetType(Data);
>    uint8_t Info = (Binding << ELF_STB_Shift) | (Type << ELF_STT_Shift);
>
> -  // Other and Visibility share the same byte with Visability using the lower
> +  // Other and Visibility share the same byte with Visibility using the lower
>    // 2 bits
>    uint8_t Visibility = MCELF::GetVisibility(OrigData);
>    uint8_t Other = MCELF::getOther(OrigData) <<
> @@ -590,8 +587,15 @@ void ELFObjectWriter::WriteSymbolTable(M
>    // The first entry is the undefined symbol entry.
>    WriteSymbolEntry(SymtabF, ShndxF, 0, 0, 0, 0, 0, 0, false);
>
> +  for (unsigned i = 0, e = FileSymbolData.size(); i != e; ++i) {
> +    WriteSymbolEntry(SymtabF, ShndxF, FileSymbolData[i],
> +                     ELF::STT_FILE | ELF::STB_LOCAL, 0, 0,
> +                     ELF::STV_DEFAULT, ELF::SHN_ABS, true);
> +  }
> +
>    // Write the symbol table entries.
> -  LastLocalSymbolIndex = LocalSymbolData.size() + 1;
> +  LastLocalSymbolIndex = FileSymbolData.size() + LocalSymbolData.size() + 1;
> +
>    for (unsigned i = 0, e = LocalSymbolData.size(); i != e; ++i) {
>      ELFSymbolData &MSD = LocalSymbolData[i];
>      WriteSymbol(SymtabF, ShndxF, MSD, Layout);
> @@ -880,6 +884,20 @@ void ELFObjectWriter::ComputeSymbolTable
>    // FIXME: We could optimize suffixes in strtab in the same way we
>    // optimize them in shstrtab.
>
> +  for (MCAssembler::const_file_name_iterator it = Asm.file_names_begin(),
> +                                            ie = Asm.file_names_end();
> +                                            it != ie;
> +                                            ++it) {
> +    StringRef Name = *it;
> +    uint64_t &Entry = StringIndexMap[Name];
> +    if (!Entry) {
> +      Entry = StringTable.size();
> +      StringTable += Name;
> +      StringTable += '\x00';
> +    }
> +    FileSymbolData.push_back(Entry);
> +  }
> +
>    // Add the data for the symbols.
>    for (MCAssembler::symbol_iterator it = Asm.symbol_begin(),
>           ie = Asm.symbol_end(); it != ie; ++it) {
> @@ -964,7 +982,7 @@ void ELFObjectWriter::ComputeSymbolTable
>
>    // Set the symbol indices. Local symbols must come before all other
>    // symbols with non-local bindings.
> -  unsigned Index = 1;
> +  unsigned Index = FileSymbolData.size() + 1;
>    for (unsigned i = 0, e = LocalSymbolData.size(); i != e; ++i)
>      LocalSymbolData[i].SymbolData->setIndex(Index++);
>
> @@ -1073,7 +1091,7 @@ void ELFObjectWriter::WriteRelocationsFr
>      else if (entry.Index < 0)
>        entry.Index = getSymbolIndexInSymbolTable(Asm, entry.Symbol);
>      else
> -      entry.Index += LocalSymbolData.size();
> +      entry.Index += FileSymbolData.size() + LocalSymbolData.size();
>      if (is64Bit()) {
>        String64(*F, entry.r_offset);
>        if (TargetObjectWriter->isN64()) {
>
> Modified: llvm/trunk/lib/MC/MCELF.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELF.cpp?rev=193569&r1=193568&r2=193569&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCELF.cpp (original)
> +++ llvm/trunk/lib/MC/MCELF.cpp Mon Oct 28 20:06:17 2013
> @@ -36,8 +36,8 @@ unsigned MCELF::GetBinding(const MCSymbo
>  void MCELF::SetType(MCSymbolData &SD, unsigned Type) {
>    assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT ||
>           Type == ELF::STT_FUNC || Type == ELF::STT_SECTION ||
> -         Type == ELF::STT_FILE || Type == ELF::STT_COMMON ||
> -         Type == ELF::STT_TLS || Type == ELF::STT_GNU_IFUNC);
> +         Type == ELF::STT_COMMON || Type == ELF::STT_TLS ||
> +         Type == ELF::STT_GNU_IFUNC);
>
>    uint32_t OtherFlags = SD.getFlags() & ~(0xf << ELF_STT_Shift);
>    SD.setFlags(OtherFlags | (Type << ELF_STT_Shift));
> @@ -47,8 +47,7 @@ unsigned MCELF::GetType(const MCSymbolDa
>    uint32_t Type = (SD.getFlags() & (0xf << ELF_STT_Shift)) >> ELF_STT_Shift;
>    assert(Type == ELF::STT_NOTYPE || Type == ELF::STT_OBJECT ||
>           Type == ELF::STT_FUNC || Type == ELF::STT_SECTION ||
> -         Type == ELF::STT_FILE || Type == ELF::STT_COMMON ||
> -         Type == ELF::STT_TLS || Type == ELF::STT_GNU_IFUNC);
> +         Type == ELF::STT_COMMON || Type == ELF::STT_TLS || Type == ELF::STT_GNU_IFUNC);
>    return Type;
>  }
>
>
> Modified: llvm/trunk/lib/MC/MCELFStreamer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCELFStreamer.cpp?rev=193569&r1=193568&r2=193569&view=diff
> ==============================================================================
> --- llvm/trunk/lib/MC/MCELFStreamer.cpp (original)
> +++ llvm/trunk/lib/MC/MCELFStreamer.cpp Mon Oct 28 20:06:17 2013
> @@ -316,17 +316,11 @@ void MCELFStreamer::EmitValueToAlignment
>                                           ValueSize, MaxBytesToEmit);
>  }
>
> -
> -// Add a symbol for the file name of this module. This is the second
> -// entry in the module's symbol table (the first being the null symbol).
> +// Add a symbol for the file name of this module. They start after the
> +// null symbol and don't count as normal symbol, i.e. a non-STT_FILE symbol
> +// with the same name may appear.
>  void MCELFStreamer::EmitFileDirective(StringRef Filename) {
> -  MCSymbol *Symbol = getAssembler().getContext().GetOrCreateSymbol(Filename);
> -  Symbol->setSection(*getCurrentSection().first);
> -  Symbol->setAbsolute();
> -
> -  MCSymbolData &SD = getAssembler().getOrCreateSymbolData(*Symbol);
> -
> -  SD.setFlags(ELF_STT_File | ELF_STB_Local | ELF_STV_Default);
> +  getAssembler().addFileName(Filename);
>  }
>
>  void MCELFStreamer::EmitIdent(StringRef IdentString) {
>
> Added: llvm/trunk/test/MC/ELF/file-double.s
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ELF/file-double.s?rev=193569&view=auto
> ==============================================================================
> --- llvm/trunk/test/MC/ELF/file-double.s (added)
> +++ llvm/trunk/test/MC/ELF/file-double.s Mon Oct 28 20:06:17 2013
> @@ -0,0 +1,47 @@
> +// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o - | llvm-readobj -t | FileCheck %s
> +
> +// Test that a STT_FILE symbol and a symbol of the same name can coexist.
> +
> +.file "foo.c"
> +.file "bar.c"
> +       .globl foo.c
> +foo.c:
> +
> +       .globl bar.c
> +bar.c:
> +
> +// CHECK:        Symbol {
> +// CHECK:          Name: foo.c (1)
> +// CHECK-NEXT:     Value: 0x0
> +// CHECK-NEXT:     Size: 0
> +// CHECK-NEXT:     Binding: Local
> +// CHECK-NEXT:     Type: File
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: (0xFFF1)
> +// CHECK-NEXT:   }
> +// CHECK:          Name: bar.c (7)
> +// CHECK-NEXT:     Value: 0x0
> +// CHECK-NEXT:     Size: 0
> +// CHECK-NEXT:     Binding: Local
> +// CHECK-NEXT:     Type: File
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: (0xFFF1)
> +// CHECK-NEXT:   }
> +// CHECK:        Symbol {
> +// CHECK:        Name: bar.c (7)
> +// CHECK-NEXT:     Value: 0x0
> +// CHECK-NEXT:     Size: 0
> +// CHECK-NEXT:     Binding: Global
> +// CHECK-NEXT:     Type: None
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: .text (0x1)
> +// CHECK-NEXT:   }
> +// CHECK:        Symbol {
> +// CHECK:        Name: foo.c (1)
> +// CHECK-NEXT:     Value: 0x0
> +// CHECK-NEXT:     Size: 0
> +// CHECK-NEXT:     Binding: Global
> +// CHECK-NEXT:     Type: None
> +// CHECK-NEXT:     Other: 0
> +// CHECK-NEXT:     Section: .text (0x1)
> +// 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