[lldb] Add support for sorting by size to target module dump symtab

[JDevlieghere]

This patch adds support to sort the symbol table by size. The command already supports sorting and it already reports sizes. Sorting by size helps diagnosing size issues.

rdar://123788375

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This patch adds support to sort the symbol table by size. The command already supports sorting and it already reports sizes. Sorting by size helps diagnosing size issues.

rdar://123788375

---

Full diff: https://github.com/llvm/llvm-project/pull/83527.diff

4 Files Affected:

• (modified) lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h (+5)
• (modified) lldb/include/lldb/lldb-private-enumerations.h (+20-15)
• (modified) lldb/source/Symbol/Symtab.cpp (+17-6)
• (added) lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test (+11)

diff --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
index 9248e2ac814461..b5e989633ea3fc 100644
--- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
+++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
@@ -50,6 +50,11 @@ static constexpr OptionEnumValueElement g_sort_option_enumeration[] = {
         "name",
         "Sort output by symbol name.",
     },
+    {
+        eSortOrderBySize,
+        "size",
+        "Sort output by symbol byte size.",
+    },
 };
 
 // Note that the negation in the argument name causes a slightly confusing
diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h
index 9e8ab56305bef3..7649fe00a84ad4 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -108,7 +108,12 @@ enum ArgumentRepetitionType {
                               // optional
 };
 
-enum SortOrder { eSortOrderNone, eSortOrderByAddress, eSortOrderByName };
+enum SortOrder {
+  eSortOrderNone,
+  eSortOrderByAddress,
+  eSortOrderByName,
+  eSortOrderBySize
+};
 
 // LazyBool is for boolean values that need to be calculated lazily. Values
 // start off set to eLazyBoolCalculate, and then they can be calculated once
@@ -236,19 +241,19 @@ enum LoadDependentFiles {
 };
 
 inline std::string GetStatDescription(lldb_private::StatisticKind K) {
-   switch (K) {
-   case StatisticKind::ExpressionSuccessful:
-     return "Number of expr evaluation successes";
-   case StatisticKind::ExpressionFailure:
-     return "Number of expr evaluation failures";
-   case StatisticKind::FrameVarSuccess:
-     return "Number of frame var successes";
-   case StatisticKind::FrameVarFailure:
-     return "Number of frame var failures";
-   case StatisticKind::StatisticMax:
-     return "";
-   }
-   llvm_unreachable("Statistic not registered!");
+  switch (K) {
+  case StatisticKind::ExpressionSuccessful:
+    return "Number of expr evaluation successes";
+  case StatisticKind::ExpressionFailure:
+    return "Number of expr evaluation failures";
+  case StatisticKind::FrameVarSuccess:
+    return "Number of frame var successes";
+  case StatisticKind::FrameVarFailure:
+    return "Number of frame var failures";
+  case StatisticKind::StatisticMax:
+    return "";
+  }
+  llvm_unreachable("Statistic not registered!");
 }
 
 } // namespace lldb_private
@@ -271,7 +276,7 @@ template <> struct format_provider<lldb_private::Vote> {
     Stream << "invalid";
   }
 };
-}
+} // namespace llvm
 
 enum SelectMostRelevant : bool {
   SelectMostRelevantFrame = true,
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 564a3a94cfa202..b7837892d7e26d 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -124,12 +124,8 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder sort_order,
       DumpSymbolHeader(s);
 
       std::multimap<llvm::StringRef, const Symbol *> name_map;
-      for (const_iterator pos = m_symbols.begin(), end = m_symbols.end();
-           pos != end; ++pos) {
-        const char *name = pos->GetName().AsCString();
-        if (name && name[0])
-          name_map.insert(std::make_pair(name, &(*pos)));
-      }
+      for (const Symbol &symbol : m_symbols)
+        name_map.emplace(llvm::StringRef(symbol.GetName()), &symbol);
 
       for (const auto &name_to_symbol : name_map) {
         const Symbol *symbol = name_to_symbol.second;
@@ -138,6 +134,21 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder sort_order,
       }
     } break;
 
+    case eSortOrderBySize: {
+      s->PutCString(" (sorted by size):\n");
+      DumpSymbolHeader(s);
+
+      std::multimap<size_t, const Symbol *, std::greater<size_t>> size_map;
+      for (const Symbol &symbol : m_symbols)
+        size_map.emplace(symbol.GetByteSize(), &symbol);
+
+      for (const auto &size_to_symbol : size_map) {
+        const Symbol *symbol = size_to_symbol.second;
+        s->Indent();
+        symbol->Dump(s, target, symbol - &m_symbols[0], name_preference);
+      }
+    } break;
+
     case eSortOrderByAddress:
       s->PutCString(" (sorted by address):\n");
       DumpSymbolHeader(s);
diff --git a/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
new file mode 100644
index 00000000000000..a9b6c0b1ef09b0
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
@@ -0,0 +1,11 @@
+# RUN: yaml2obj %S/Inputs/basic-elf.yaml -o %T/symtab.out
+# RUN: %lldb %T/symtab.out -o "target symbols add -s symtab.out %S/Inputs/symtab.syms" \
+# RUN:   -s %s | FileCheck %s
+
+# CHECK: num_symbols = 4 (sorted by size):
+# CHECK: [    0]      0  SX Code            0x0000000000400000                    0x00000000000000b0 0x00000000 ___lldb_unnamed_symbol0
+# CHECK: [    3]      0   X Code            0x00000000004000d0                    0x0000000000000022 0x00000000 _start
+# CHECK: [    1]      0   X Code            0x00000000004000b0                    0x0000000000000010 0x00000000 f1
+# CHECK: [    2]      0   X Code            0x00000000004000c0                    0x0000000000000010 0x00000000 f2
+
+image dump symtab -s size symtab.out

[medismailben]

LGTM!

[jasonmolenda]

I don't have a strong opinion here but when it's sorted by address, the index is still sorted-ordering. This is showing the original symtab index numbers. I think it should show the sorted index ordering, if someone wants to correspond a symbol to the original symtab, they can search for the symbol name.

[JDevlieghere]

#83889