From a3ccf3ddbd12a0db6522f9990a9e30e535ca76dc Mon Sep 17 00:00:00 2001
From: Kuba Brecka <kuba.brecka@gmail.com>
Date: Tue, 15 Nov 2016 21:07:03 +0000
Subject: [PATCH] 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

llvm-svn: 287028
---
 llvm/include/llvm/Object/SymbolSize.h    | 11 ++++++++
 llvm/lib/Object/SymbolSize.cpp           | 18 +++++--------
 llvm/unittests/CMakeLists.txt            |  1 +
 llvm/unittests/Object/CMakeLists.txt     |  8 ++++++
 llvm/unittests/Object/SymbolSizeTest.cpp | 33 ++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 12 deletions(-)
 create mode 100644 llvm/unittests/Object/CMakeLists.txt
 create mode 100644 llvm/unittests/Object/SymbolSizeTest.cpp

diff --git a/llvm/include/llvm/Object/SymbolSize.h b/llvm/include/llvm/Object/SymbolSize.h
index f2ce70f4208d..1a1dc8752943 100644
--- a/llvm/include/llvm/Object/SymbolSize.h
+++ b/llvm/include/llvm/Object/SymbolSize.h
@@ -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
 
diff --git a/llvm/lib/Object/SymbolSize.cpp b/llvm/lib/Object/SymbolSize.cpp
index 1d5cd78e6d9c..dd49d5f116b3 100644
--- a/llvm/lib/Object/SymbolSize.cpp
+++ b/llvm/lib/Object/SymbolSize.cpp
@@ -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) {
diff --git a/llvm/unittests/CMakeLists.txt b/llvm/unittests/CMakeLists.txt
index 6caed0e5d199..8dbca211d026 100644
--- a/llvm/unittests/CMakeLists.txt
+++ b/llvm/unittests/CMakeLists.txt
@@ -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)
diff --git a/llvm/unittests/Object/CMakeLists.txt b/llvm/unittests/Object/CMakeLists.txt
new file mode 100644
index 000000000000..7a63c167a30b
--- /dev/null
+++ b/llvm/unittests/Object/CMakeLists.txt
@@ -0,0 +1,8 @@
+set(LLVM_LINK_COMPONENTS
+  Object
+  )
+
+add_llvm_unittest(ObjectTests
+  SymbolSizeTest.cpp
+  )
+
diff --git a/llvm/unittests/Object/SymbolSizeTest.cpp b/llvm/unittests/Object/SymbolSizeTest.cpp
new file mode 100644
index 000000000000..ad9c40b986db
--- /dev/null
+++ b/llvm/unittests/Object/SymbolSizeTest.cpp
@@ -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);
+  }
+}