[llvm] r238028 - Stop inventing symbol sizes.

Rafael Espindola rafael.espindola at gmail.com
Fri May 22 08:43:00 PDT 2015


Author: rafael
Date: Fri May 22 10:43:00 2015
New Revision: 238028

URL: http://llvm.org/viewvc/llvm-project?rev=238028&view=rev
Log:
Stop inventing symbol sizes.

MachO and COFF quite reasonably only define the size for common symbols.

We used to try to figure out the "size" by computing the gap from one symbol to
the next.

This would not be correct in general, since a part of a section can belong to no
visible symbol (padding, private globals).

It was also really expensive, since we would walk every symbol to find the size
of one.

If a caller really wants this, it can sort all the symbols once and get all the
gaps ("size") in O(n log n) instead of O(n^2).

On MachO this also has the advantage of centralizing all the checks for an
invalid n_sect.

Removed:
    llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index
    llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index
Modified:
    llvm/trunk/lib/Object/COFFObjectFile.cpp
    llvm/trunk/lib/Object/MachOObjectFile.cpp
    llvm/trunk/test/Object/macho-invalid.test
    llvm/trunk/test/Object/objdump-symbol-table.test
    llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test

Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=238028&r1=238027&r2=238028&view=diff
==============================================================================
--- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/COFFObjectFile.cpp Fri May 22 10:43:00 2015
@@ -240,59 +240,10 @@ std::error_code COFFObjectFile::getSymbo
                                               uint64_t &Result) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
 
-  if (Symb.isAnyUndefined()) {
-    Result = UnknownAddressOrSize;
-    return object_error::success;
-  }
-  if (Symb.isCommon()) {
+  if (Symb.isCommon())
     Result = Symb.getValue();
-    return object_error::success;
-  }
-
-  // Let's attempt to get the size of the symbol by looking at the address of
-  // the symbol after the symbol in question.
-  uint64_t SymbAddr;
-  if (std::error_code EC = getSymbolAddress(Ref, SymbAddr))
-    return EC;
-  int32_t SectionNumber = Symb.getSectionNumber();
-  if (COFF::isReservedSectionNumber(SectionNumber)) {
-    // Absolute and debug symbols aren't sorted in any interesting way.
-    Result = 0;
-    return object_error::success;
-  }
-  const section_iterator SecEnd = section_end();
-  uint64_t AfterAddr = UnknownAddressOrSize;
-  for (const symbol_iterator SymbI : symbols()) {
-    section_iterator SecI = SecEnd;
-    if (std::error_code EC = SymbI->getSection(SecI))
-      return EC;
-    // Check the symbol's section, skip it if it's in the wrong section.
-    // First, make sure it is in any section.
-    if (SecI == SecEnd)
-      continue;
-    // Second, make sure it is in the same section as the symbol in question.
-    if (!sectionContainsSymbol(SecI->getRawDataRefImpl(), Ref))
-      continue;
-    uint64_t Addr;
-    if (std::error_code EC = SymbI->getAddress(Addr))
-      return EC;
-    // We want to compare our symbol in question with the closest possible
-    // symbol that comes after.
-    if (AfterAddr > Addr && Addr > SymbAddr)
-      AfterAddr = Addr;
-  }
-  if (AfterAddr == UnknownAddressOrSize) {
-    // No symbol comes after this one, assume that everything after our symbol
-    // is part of it.
-    const coff_section *Section = nullptr;
-    if (std::error_code EC = getSection(SectionNumber, Section))
-      return EC;
-    Result = Section->SizeOfRawData - Symb.getValue();
-  } else {
-    // Take the difference between our symbol and the symbol that comes after
-    // our symbol.
-    Result = AfterAddr - SymbAddr;
-  }
+  else
+    Result = UnknownAddressOrSize;
 
   return object_error::success;
 }

Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=238028&r1=238027&r2=238028&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Fri May 22 10:43:00 2015
@@ -415,49 +415,13 @@ std::error_code MachOObjectFile::getSymb
 
 std::error_code MachOObjectFile::getSymbolSize(DataRefImpl DRI,
                                                uint64_t &Result) const {
-  uint64_t BeginOffset;
-  uint64_t EndOffset = 0;
-  uint8_t SectionIndex;
-
-  MachO::nlist_base Entry = getSymbolTableEntryBase(this, DRI);
   uint64_t Value;
   getSymbolAddress(DRI, Value);
-  if (Value == UnknownAddressOrSize) {
+  uint32_t flags = getSymbolFlags(DRI);
+  if (flags & SymbolRef::SF_Common)
+    Result = Value;
+  else
     Result = UnknownAddressOrSize;
-    return object_error::success;
-  }
-
-  BeginOffset = Value;
-
-  SectionIndex = Entry.n_sect;
-  if (!SectionIndex) {
-    uint32_t flags = getSymbolFlags(DRI);
-    if (flags & SymbolRef::SF_Common)
-      Result = Value;
-    else
-      Result = UnknownAddressOrSize;
-    return object_error::success;
-  }
-  // Unfortunately symbols are unsorted so we need to touch all
-  // symbols from load command
-  for (const SymbolRef &Symbol : symbols()) {
-    DataRefImpl DRI = Symbol.getRawDataRefImpl();
-    Entry = getSymbolTableEntryBase(this, DRI);
-    getSymbolAddress(DRI, Value);
-    if (Value == UnknownAddressOrSize)
-      continue;
-    if (Entry.n_sect == SectionIndex && Value > BeginOffset)
-      if (!EndOffset || Value < EndOffset)
-        EndOffset = Value;
-  }
-  if (!EndOffset) {
-    DataRefImpl Sec;
-    Sec.d.a = SectionIndex-1;
-    uint64_t Size = getSectionSize(Sec);
-    EndOffset = getSectionAddress(Sec);
-    EndOffset += Size;
-  }
-  Result = EndOffset - BeginOffset;
   return object_error::success;
 }
 
@@ -2263,16 +2227,12 @@ MachOObjectFile::getNextLoadCommandInfo(
 }
 
 MachO::section MachOObjectFile::getSection(DataRefImpl DRI) const {
-  // TODO: What if Sections.size() == 0?
-  if (DRI.d.a >= Sections.size())
-    report_fatal_error("getSection: Invalid section index.");
+  assert(DRI.d.a < Sections.size() && "Should have detected this earlier");
   return getStruct<MachO::section>(this, Sections[DRI.d.a]);
 }
 
 MachO::section_64 MachOObjectFile::getSection64(DataRefImpl DRI) const {
-  // TODO: What if Sections.size() == 0?
-  if (DRI.d.a >= Sections.size())
-    report_fatal_error("getSection64: Invalid section index.");
+  assert(DRI.d.a < Sections.size() && "Should have detected this earlier");
   return getStruct<MachO::section_64>(this, Sections[DRI.d.a]);
 }
 

Removed: llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index?rev=238027&view=auto
==============================================================================
Binary files llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index (original) and llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index (removed) differ

Removed: llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index?rev=238027&view=auto
==============================================================================
Binary files llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index (original) and llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index (removed) differ

Modified: llvm/trunk/test/Object/macho-invalid.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/macho-invalid.test?rev=238028&r1=238027&r2=238028&view=diff
==============================================================================
--- llvm/trunk/test/Object/macho-invalid.test (original)
+++ llvm/trunk/test/Object/macho-invalid.test Fri May 22 10:43:00 2015
@@ -28,13 +28,6 @@ RUN:      | FileCheck -check-prefix NAME
 RUN: not llvm-nm %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \
 RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC %s
 
-RUN: not llvm-objdump -t %p/Inputs/macho-invalid-getsection-index 2>&1 \
-RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-GETSECT %s
-
-RUN: not llvm-objdump -t %p/Inputs/macho64-invalid-getsection-index 2>&1 \
-RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-GETSECT64 %s
-
-
 SMALL-LOADC-SIZE: Load command with size < 8 bytes
 SMALL-SEGLOADC-SIZE: Segment load command size is too small
 INCOMPLETE-LOADC: Malformed MachO file
@@ -43,5 +36,3 @@ BAD-SYMBOL: Requested symbol index is ou
 NAME-PAST-EOF: Symbol name entry points before beginning or past end of file
 
 INVALID-SECTION-IDX-SYMBOL-SEC: getSymbolSection: Invalid section index
-INVALID-SECTION-IDX-GETSECT: getSection: Invalid section index
-INVALID-SECTION-IDX-GETSECT64: getSection64: Invalid section index

Modified: llvm/trunk/test/Object/objdump-symbol-table.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/objdump-symbol-table.test?rev=238028&r1=238027&r2=238028&view=diff
==============================================================================
--- llvm/trunk/test/Object/objdump-symbol-table.test (original)
+++ llvm/trunk/test/Object/objdump-symbol-table.test Fri May 22 10:43:00 2015
@@ -30,7 +30,7 @@ ELF-i386: 00000000         *UND*  000000
 
 macho-i386: trivial-object-test.macho-i386:        file format Mach-O 32-bit i386
 macho-i386: SYMBOL TABLE:
-macho-i386: 00000000 g     F __TEXT,__text  00000024 _main
+macho-i386: 00000000 g     F __TEXT,__text  00000000 _main
 macho-i386: 00000000         *UND*  00000000 _SomeOtherFunction
 macho-i386: 00000000         *UND*  00000000 _puts
 

Modified: llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test?rev=238028&r1=238027&r2=238028&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test (original)
+++ llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test Fri May 22 10:43:00 2015
@@ -1,8 +1,8 @@
 RUN: llvm-objdump -macho -t %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s
 
 CHECK: SYMBOL TABLE:
-CHECK: 000000000000003b l     F __TEXT,__cstring	0000000d L_.str
-CHECK: 0000000000000068 l     F __TEXT,__eh_frame	00000018 EH_frame0
-CHECK: 0000000000000000 g     F __TEXT,__text	0000003b _main
-CHECK: 0000000000000080 g     F __TEXT,__eh_frame	00000028 _main.eh
+CHECK: 000000000000003b l     F __TEXT,__cstring	00000000 L_.str
+CHECK: 0000000000000068 l     F __TEXT,__eh_frame	00000000 EH_frame0
+CHECK: 0000000000000000 g     F __TEXT,__text	00000000 _main
+CHECK: 0000000000000080 g     F __TEXT,__eh_frame	00000000 _main.eh
 CHECK: 0000000000000000         *UND*	00000000 _printf





More information about the llvm-commits mailing list