[llvm] r238028 - Stop inventing symbol sizes.
Sean Silva
chisophugis at gmail.com
Fri May 22 14:33:41 PDT 2015
On Fri, May 22, 2015 at 8:43 AM, Rafael Espindola <
rafael.espindola at gmail.com> wrote:
> 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).
>
Yeah, I've run into this before. It was a totally deceptive API.
-- Sean Silva
>
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150522/d801e3c5/attachment.html>
More information about the llvm-commits
mailing list