[llvm] r287028 - Fix llvm-symbolizer to correctly sort a symbol array and calculate symbol sizes

[Kuba Brecka via llvm-commits]

Author: kuba.brecka
Date: Tue Nov 15 15:07:03 2016
New Revision: 287028

URL: http://llvm.org/viewvc/llvm-project?rev=287028&view=rev
Log:
Fix llvm-symbolizer to correctly sort a symbol array and calculate symbol sizes

Sometimes, llvm-symbolizer gives wrong results due to incorrect sizes of some symbols. The reason for that was an incorrectly sorted array in computeSymbolSizes. The comparison function used subtraction of unsigned types, which is incorrect. Let's change this to return explicit -1 or 1.

Differential Revision: https://reviews.llvm.org/D26537


Added:
    llvm/trunk/unittests/Object/
    llvm/trunk/unittests/Object/CMakeLists.txt
    llvm/trunk/unittests/Object/SymbolSizeTest.cpp
Modified:
    llvm/trunk/include/llvm/Object/SymbolSize.h
    llvm/trunk/lib/Object/SymbolSize.cpp
    llvm/trunk/unittests/CMakeLists.txt

Modified: llvm/trunk/include/llvm/Object/SymbolSize.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/SymbolSize.h?rev=287028&r1=287027&r2=287028&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/SymbolSize.h (original)
+++ llvm/trunk/include/llvm/Object/SymbolSize.h Tue Nov 15 15:07:03 2016
@@ -15,8 +15,19 @@
 
 namespace llvm {
 namespace object {
+
+struct SymEntry {
+  symbol_iterator I;
+  uint64_t Address;
+  unsigned Number;
+  unsigned SectionID;
+};
+
+int compareAddress(const SymEntry *A, const SymEntry *B);
+
 std::vector<std::pair<SymbolRef, uint64_t>>
 computeSymbolSizes(const ObjectFile &O);
+
 }
 } // namespace llvm
 

Modified: llvm/trunk/lib/Object/SymbolSize.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/SymbolSize.cpp?rev=287028&r1=287027&r2=287028&view=diff
==============================================================================
--- llvm/trunk/lib/Object/SymbolSize.cpp (original)
+++ llvm/trunk/lib/Object/SymbolSize.cpp Tue Nov 15 15:07:03 2016
@@ -16,19 +16,13 @@
 using namespace llvm;
 using namespace object;
 
-namespace {
-struct SymEntry {
-  symbol_iterator I;
-  uint64_t Address;
-  unsigned Number;
-  unsigned SectionID;
-};
-}
-
-static int compareAddress(const SymEntry *A, const SymEntry *B) {
+// Orders increasingly by (SectionID, Address).
+int llvm::object::compareAddress(const SymEntry *A, const SymEntry *B) {
   if (A->SectionID != B->SectionID)
-    return A->SectionID - B->SectionID;
-  return A->Address - B->Address;
+    return A->SectionID < B->SectionID ? -1 : 1;
+  if (A->Address != B->Address)
+    return A->Address < B->Address ? -1 : 1;
+  return 0;
 }
 
 static unsigned getSectionID(const ObjectFile &O, SectionRef Sec) {

Modified: llvm/trunk/unittests/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CMakeLists.txt?rev=287028&r1=287027&r2=287028&view=diff
==============================================================================
--- llvm/trunk/unittests/CMakeLists.txt (original)
+++ llvm/trunk/unittests/CMakeLists.txt Tue Nov 15 15:07:03 2016
@@ -17,6 +17,7 @@ add_subdirectory(LineEditor)
 add_subdirectory(Linker)
 add_subdirectory(MC)
 add_subdirectory(MI)
+add_subdirectory(Object)
 add_subdirectory(ObjectYAML)
 add_subdirectory(Option)
 add_subdirectory(ProfileData)

Added: llvm/trunk/unittests/Object/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Object/CMakeLists.txt?rev=287028&view=auto
==============================================================================
--- llvm/trunk/unittests/Object/CMakeLists.txt (added)
+++ llvm/trunk/unittests/Object/CMakeLists.txt Tue Nov 15 15:07:03 2016
@@ -0,0 +1,8 @@
+set(LLVM_LINK_COMPONENTS
+  Object
+  )
+
+add_llvm_unittest(ObjectTests
+  SymbolSizeTest.cpp
+  )
+

Added: llvm/trunk/unittests/Object/SymbolSizeTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Object/SymbolSizeTest.cpp?rev=287028&view=auto
==============================================================================
--- llvm/trunk/unittests/Object/SymbolSizeTest.cpp (added)
+++ llvm/trunk/unittests/Object/SymbolSizeTest.cpp Tue Nov 15 15:07:03 2016
@@ -0,0 +1,33 @@
+//===- SymbolSizeTest.cpp - Tests for SymbolSize.cpp ----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Object/SymbolSize.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::object;
+
+TEST(Object, SymbolSizeSort) {
+  auto it = symbol_iterator(SymbolRef());
+  std::vector<SymEntry> Syms{
+      SymEntry{it, 0xffffffff00000000ull, 1, 0},
+      SymEntry{it, 0x00ffffff00000000ull, 2, 0},
+      SymEntry{it, 0x00ffffff000000ffull, 3, 0},
+      SymEntry{it, 0x0000000100000000ull, 4, 0},
+      SymEntry{it, 0x00000000000000ffull, 5, 0},
+      SymEntry{it, 0x00000001000000ffull, 6, 0},
+      SymEntry{it, 0x000000010000ffffull, 7, 0},
+  };
+
+  array_pod_sort(Syms.begin(), Syms.end(), compareAddress);
+
+  for (unsigned I = 0, N = Syms.size(); I < N - 1; ++I) {
+    EXPECT_LE(Syms[I].Address, Syms[I + 1].Address);
+  }
+}