<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 22, 2015 at 8:43 AM, Rafael Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Fri May 22 10:43:00 2015<br>
New Revision: 238028<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D238028-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=mI-e643hIHxh1wmotXsWjlaPFFCkJlRu3Mhcz3qKpmY&e=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=238028&view=rev</a><br>
Log:<br>
Stop inventing symbol sizes.<br>
<br>
MachO and COFF quite reasonably only define the size for common symbols.<br>
<br>
We used to try to figure out the "size" by computing the gap from one symbol to<br>
the next.<br>
<br>
This would not be correct in general, since a part of a section can belong to no<br>
visible symbol (padding, private globals).<br>
<br>
It was also really expensive, since we would walk every symbol to find the size<br>
of one.<br>
<br>
If a caller really wants this, it can sort all the symbols once and get all the<br>
gaps ("size") in O(n log n) instead of O(n^2).<br></blockquote><div><br></div><div>Yeah, I've run into this before. It was a totally deceptive API.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On MachO this also has the advantage of centralizing all the checks for an<br>
invalid n_sect.<br>
<br>
Removed:<br>
    llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index<br>
    llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index<br>
Modified:<br>
    llvm/trunk/lib/Object/COFFObjectFile.cpp<br>
    llvm/trunk/lib/Object/MachOObjectFile.cpp<br>
    llvm/trunk/test/Object/macho-invalid.test<br>
    llvm/trunk/test/Object/objdump-symbol-table.test<br>
    llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test<br>
<br>
Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Object_COFFObjectFile.cpp-3Frev-3D238028-26r1-3D238027-26r2-3D238028-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=IC2hIdZh4xwGqDWLySgzyczsYk058Bu8GHlax9dDOX8&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=238028&r1=238027&r2=238028&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)<br>
+++ llvm/trunk/lib/Object/COFFObjectFile.cpp Fri May 22 10:43:00 2015<br>
@@ -240,59 +240,10 @@ std::error_code COFFObjectFile::getSymbo<br>
                                               uint64_t &Result) const {<br>
   COFFSymbolRef Symb = getCOFFSymbol(Ref);<br>
<br>
-  if (Symb.isAnyUndefined()) {<br>
-    Result = UnknownAddressOrSize;<br>
-    return object_error::success;<br>
-  }<br>
-  if (Symb.isCommon()) {<br>
+  if (Symb.isCommon())<br>
     Result = Symb.getValue();<br>
-    return object_error::success;<br>
-  }<br>
-<br>
-  // Let's attempt to get the size of the symbol by looking at the address of<br>
-  // the symbol after the symbol in question.<br>
-  uint64_t SymbAddr;<br>
-  if (std::error_code EC = getSymbolAddress(Ref, SymbAddr))<br>
-    return EC;<br>
-  int32_t SectionNumber = Symb.getSectionNumber();<br>
-  if (COFF::isReservedSectionNumber(SectionNumber)) {<br>
-    // Absolute and debug symbols aren't sorted in any interesting way.<br>
-    Result = 0;<br>
-    return object_error::success;<br>
-  }<br>
-  const section_iterator SecEnd = section_end();<br>
-  uint64_t AfterAddr = UnknownAddressOrSize;<br>
-  for (const symbol_iterator SymbI : symbols()) {<br>
-    section_iterator SecI = SecEnd;<br>
-    if (std::error_code EC = SymbI->getSection(SecI))<br>
-      return EC;<br>
-    // Check the symbol's section, skip it if it's in the wrong section.<br>
-    // First, make sure it is in any section.<br>
-    if (SecI == SecEnd)<br>
-      continue;<br>
-    // Second, make sure it is in the same section as the symbol in question.<br>
-    if (!sectionContainsSymbol(SecI->getRawDataRefImpl(), Ref))<br>
-      continue;<br>
-    uint64_t Addr;<br>
-    if (std::error_code EC = SymbI->getAddress(Addr))<br>
-      return EC;<br>
-    // We want to compare our symbol in question with the closest possible<br>
-    // symbol that comes after.<br>
-    if (AfterAddr > Addr && Addr > SymbAddr)<br>
-      AfterAddr = Addr;<br>
-  }<br>
-  if (AfterAddr == UnknownAddressOrSize) {<br>
-    // No symbol comes after this one, assume that everything after our symbol<br>
-    // is part of it.<br>
-    const coff_section *Section = nullptr;<br>
-    if (std::error_code EC = getSection(SectionNumber, Section))<br>
-      return EC;<br>
-    Result = Section->SizeOfRawData - Symb.getValue();<br>
-  } else {<br>
-    // Take the difference between our symbol and the symbol that comes after<br>
-    // our symbol.<br>
-    Result = AfterAddr - SymbAddr;<br>
-  }<br>
+  else<br>
+    Result = UnknownAddressOrSize;<br>
<br>
   return object_error::success;<br>
 }<br>
<br>
Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_Object_MachOObjectFile.cpp-3Frev-3D238028-26r1-3D238027-26r2-3D238028-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=KGY9vLAgvLX68xNM8CJF6oXqLHY-89LMpieTG-HIyRc&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=238028&r1=238027&r2=238028&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)<br>
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Fri May 22 10:43:00 2015<br>
@@ -415,49 +415,13 @@ std::error_code MachOObjectFile::getSymb<br>
<br>
 std::error_code MachOObjectFile::getSymbolSize(DataRefImpl DRI,<br>
                                                uint64_t &Result) const {<br>
-  uint64_t BeginOffset;<br>
-  uint64_t EndOffset = 0;<br>
-  uint8_t SectionIndex;<br>
-<br>
-  MachO::nlist_base Entry = getSymbolTableEntryBase(this, DRI);<br>
   uint64_t Value;<br>
   getSymbolAddress(DRI, Value);<br>
-  if (Value == UnknownAddressOrSize) {<br>
+  uint32_t flags = getSymbolFlags(DRI);<br>
+  if (flags & SymbolRef::SF_Common)<br>
+    Result = Value;<br>
+  else<br>
     Result = UnknownAddressOrSize;<br>
-    return object_error::success;<br>
-  }<br>
-<br>
-  BeginOffset = Value;<br>
-<br>
-  SectionIndex = Entry.n_sect;<br>
-  if (!SectionIndex) {<br>
-    uint32_t flags = getSymbolFlags(DRI);<br>
-    if (flags & SymbolRef::SF_Common)<br>
-      Result = Value;<br>
-    else<br>
-      Result = UnknownAddressOrSize;<br>
-    return object_error::success;<br>
-  }<br>
-  // Unfortunately symbols are unsorted so we need to touch all<br>
-  // symbols from load command<br>
-  for (const SymbolRef &Symbol : symbols()) {<br>
-    DataRefImpl DRI = Symbol.getRawDataRefImpl();<br>
-    Entry = getSymbolTableEntryBase(this, DRI);<br>
-    getSymbolAddress(DRI, Value);<br>
-    if (Value == UnknownAddressOrSize)<br>
-      continue;<br>
-    if (Entry.n_sect == SectionIndex && Value > BeginOffset)<br>
-      if (!EndOffset || Value < EndOffset)<br>
-        EndOffset = Value;<br>
-  }<br>
-  if (!EndOffset) {<br>
-    DataRefImpl Sec;<br>
-    Sec.d.a = SectionIndex-1;<br>
-    uint64_t Size = getSectionSize(Sec);<br>
-    EndOffset = getSectionAddress(Sec);<br>
-    EndOffset += Size;<br>
-  }<br>
-  Result = EndOffset - BeginOffset;<br>
   return object_error::success;<br>
 }<br>
<br>
@@ -2263,16 +2227,12 @@ MachOObjectFile::getNextLoadCommandInfo(<br>
 }<br>
<br>
 MachO::section MachOObjectFile::getSection(DataRefImpl DRI) const {<br>
-  // TODO: What if Sections.size() == 0?<br>
-  if (DRI.d.a >= Sections.size())<br>
-    report_fatal_error("getSection: Invalid section index.");<br>
+  assert(DRI.d.a < Sections.size() && "Should have detected this earlier");<br>
   return getStruct<MachO::section>(this, Sections[DRI.d.a]);<br>
 }<br>
<br>
 MachO::section_64 MachOObjectFile::getSection64(DataRefImpl DRI) const {<br>
-  // TODO: What if Sections.size() == 0?<br>
-  if (DRI.d.a >= Sections.size())<br>
-    report_fatal_error("getSection64: Invalid section index.");<br>
+  assert(DRI.d.a < Sections.size() && "Should have detected this earlier");<br>
   return getStruct<MachO::section_64>(this, Sections[DRI.d.a]);<br>
 }<br>
<br>
<br>
Removed: llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Object_Inputs_macho-2Dinvalid-2Dgetsection-2Dindex-3Frev-3D238027-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=skbgsKGMyegFbE9438UfEL5OOTcJJZZpjGg1tVnkIGU&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index?rev=238027&view=auto</a><br>
==============================================================================<br>
Binary files llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index (original) and llvm/trunk/test/Object/Inputs/macho-invalid-getsection-index (removed) differ<br>
<br>
Removed: llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Object_Inputs_macho64-2Dinvalid-2Dgetsection-2Dindex-3Frev-3D238027-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=ZROJj_XbMTfIA23CBsogbFTVIorc9Ari4lAP5auIzqM&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index?rev=238027&view=auto</a><br>
==============================================================================<br>
Binary files llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index (original) and llvm/trunk/test/Object/Inputs/macho64-invalid-getsection-index (removed) differ<br>
<br>
Modified: llvm/trunk/test/Object/macho-invalid.test<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Object_macho-2Dinvalid.test-3Frev-3D238028-26r1-3D238027-26r2-3D238028-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=shFW4y19G-71h5lHL8jFC-O622fxL31HJXWadj33YRk&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/macho-invalid.test?rev=238028&r1=238027&r2=238028&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Object/macho-invalid.test (original)<br>
+++ llvm/trunk/test/Object/macho-invalid.test Fri May 22 10:43:00 2015<br>
@@ -28,13 +28,6 @@ RUN:      | FileCheck -check-prefix NAME<br>
 RUN: not llvm-nm %p/Inputs/macho-invalid-section-index-getSectionRawName 2>&1 \<br>
 RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-SYMBOL-SEC %s<br>
<br>
-RUN: not llvm-objdump -t %p/Inputs/macho-invalid-getsection-index 2>&1 \<br>
-RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-GETSECT %s<br>
-<br>
-RUN: not llvm-objdump -t %p/Inputs/macho64-invalid-getsection-index 2>&1 \<br>
-RUN:      | FileCheck -check-prefix INVALID-SECTION-IDX-GETSECT64 %s<br>
-<br>
-<br>
 SMALL-LOADC-SIZE: Load command with size < 8 bytes<br>
 SMALL-SEGLOADC-SIZE: Segment load command size is too small<br>
 INCOMPLETE-LOADC: Malformed MachO file<br>
@@ -43,5 +36,3 @@ BAD-SYMBOL: Requested symbol index is ou<br>
 NAME-PAST-EOF: Symbol name entry points before beginning or past end of file<br>
<br>
 INVALID-SECTION-IDX-SYMBOL-SEC: getSymbolSection: Invalid section index<br>
-INVALID-SECTION-IDX-GETSECT: getSection: Invalid section index<br>
-INVALID-SECTION-IDX-GETSECT64: getSection64: Invalid section index<br>
<br>
Modified: llvm/trunk/test/Object/objdump-symbol-table.test<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_Object_objdump-2Dsymbol-2Dtable.test-3Frev-3D238028-26r1-3D238027-26r2-3D238028-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=kXuNEXj1rKOxK6bGAfk8qoHl7cY7UKVuTBoy--txsHw&e=" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/objdump-symbol-table.test?rev=238028&r1=238027&r2=238028&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Object/objdump-symbol-table.test (original)<br>
+++ llvm/trunk/test/Object/objdump-symbol-table.test Fri May 22 10:43:00 2015<br>
@@ -30,7 +30,7 @@ ELF-i386: 00000000         *UND*  000000<br>
<br>
 macho-i386: trivial-object-test.macho-i386:        file format Mach-O 32-bit i386<br>
 macho-i386: SYMBOL TABLE:<br>
-macho-i386: 00000000 g     F __TEXT,__text  00000024 _main<br>
+macho-i386: 00000000 g     F __TEXT,__text  00000000 _main<br>
 macho-i386: 00000000         *UND*  00000000 _SomeOtherFunction<br>
 macho-i386: 00000000         *UND*  00000000 _puts<br>
<br>
<br>
Modified: llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_test_tools_llvm-2Dobjdump_X86_macho-2Dsymbol-2Dtable.test-3Frev-3D238028-26r1-3D238027-26r2-3D238028-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=Z_LOyV3PCnpiw07CP6pt6eNK3L9gHnuBue-GfrvaPYk&s=S0gxlCWcUnMT5GKlLk9ou6BT5qcYei6mCoQaP5Ri57E&e=" target="_blank">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</a><br>
==============================================================================<br>
--- llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test (original)<br>
+++ llvm/trunk/test/tools/llvm-objdump/X86/macho-symbol-table.test Fri May 22 10:43:00 2015<br>
@@ -1,8 +1,8 @@<br>
 RUN: llvm-objdump -macho -t %p/Inputs/hello.obj.macho-x86_64 | FileCheck %s<br>
<br>
 CHECK: SYMBOL TABLE:<br>
-CHECK: 000000000000003b l     F __TEXT,__cstring       0000000d L_.str<br>
-CHECK: 0000000000000068 l     F __TEXT,__eh_frame      00000018 EH_frame0<br>
-CHECK: 0000000000000000 g     F __TEXT,__text  0000003b _main<br>
-CHECK: 0000000000000080 g     F __TEXT,__eh_frame      00000028 _main.eh<br>
+CHECK: 000000000000003b l     F __TEXT,__cstring       00000000 L_.str<br>
+CHECK: 0000000000000068 l     F __TEXT,__eh_frame      00000000 EH_frame0<br>
+CHECK: 0000000000000000 g     F __TEXT,__text  00000000 _main<br>
+CHECK: 0000000000000080 g     F __TEXT,__eh_frame      00000000 _main.eh<br>
 CHECK: 0000000000000000         *UND*  00000000 _printf<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>