breakpad: Add FUNC records to the symtab

This patch extends SymbolFileBreakpad::AddSymbols to include the symbols
from the FUNC records too. These symbols come from the debug info and
have a size associated with them, so they are given preference in case
there is a PUBLIC record for the same address.

To achieve this, I first pre-process the symbols into a temporary
DenseMap, and then insert the uniqued symbols into the module's symtab.

Reviewers: clayborg, lemo, zturner

Reviewed By: clayborg

Subscribers: lldb-commits

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

llvm-svn: 351781

lldb/lit/SymbolFile/Breakpad/Inputs/symtab.syms

@@ -5,3 +5,5 @@ PUBLIC b0 0 f1
 PUBLIC m c0 0 f2
 PUBLIC d0 0 _start
 PUBLIC ff 0 _out_of_range_ignored
+FUNC b0 c 0 f1_func
+FUNC m a0 d 0 func_only

lldb/lit/SymbolFile/Breakpad/symtab.test

@@ -3,15 +3,16 @@
 # RUN:   -s %s | FileCheck %s
 
 # CHECK-LABEL: (lldb) image dump symtab symtab.out
-# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 3:
+# CHECK: Symtab, file = {{.*}}symtab.out, num_symbols = 4:
 # CHECK: Index   UserID DSX Type            File Address/Value Load Address       Size               Flags      Name
-# CHECK: [    0]      0   X Code            0x00000000004000b0                    0x0000000000000010 0x00000000 f1
+# CHECK: [    0]      0   X Code            0x00000000004000c0                    0x0000000000000010 0x00000000 f2
-# CHECK: [    1]      0   X Code            0x00000000004000c0                    0x0000000000000010 0x00000000 f2
+# CHECK: [    1]      0   X Code            0x00000000004000d0                    0x0000000000000022 0x00000000 _start
-# CHECK: [    2]      0   X Code            0x00000000004000d0                    0x0000000000000022 0x00000000 _start
+# CHECK: [    2]      0   X Code            0x00000000004000a0                    0x000000000000000d 0x00000000 func_only
+# CHECK: [    3]      0   X Code            0x00000000004000b0                    0x000000000000000c 0x00000000 f1_func
 
 # CHECK-LABEL: (lldb) image lookup -a 0x4000b0 -v
 # CHECK: Address: symtab.out[0x00000000004000b0] (symtab.out.PT_LOAD[0]..text2 + 0)
-# CHECK: Symbol: id = {0x00000000}, range = [0x00000000004000b0-0x00000000004000c0), name="f1"
+# CHECK: Symbol: id = {0x00000000}, range = [0x00000000004000b0-0x00000000004000bc), name="f1_func"
 
 # CHECK-LABEL: (lldb) image lookup -n f2 -v
 # CHECK: Address: symtab.out[0x00000000004000c0] (symtab.out.PT_LOAD[0]..text2 + 16)

lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp

@@ -191,32 +191,77 @@ llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
   return OS << "INFO CODE_ID " << R.getID().GetAsString();
 }
 
-llvm::Optional<PublicRecord> PublicRecord::parse(llvm::StringRef Line) {
+static bool parsePublicOrFunc(llvm::StringRef Line, bool &Multiple,
+                              lldb::addr_t &Address, lldb::addr_t *Size,
+                              lldb::addr_t &ParamSize, llvm::StringRef &Name) {
   // PUBLIC [m] address param_size name
+  // or
+  // FUNC [m] address size param_size name
+
+  Token Tok = Size ? Token::Func : Token::Public;
+
   llvm::StringRef Str;
   std::tie(Str, Line) = getToken(Line);
-  if (toToken(Str) != Token::Public)
+  if (toToken(Str) != Tok)
-    return llvm::None;
+    return false;
 
   std::tie(Str, Line) = getToken(Line);
-  bool Multiple = Str == "m";
+  Multiple = Str == "m";
 
   if (Multiple)
     std::tie(Str, Line) = getToken(Line);
-  lldb::addr_t Address;
   if (!to_integer(Str, Address, 16))
-    return llvm::None;
+    return false;
+
+  if (Tok == Token::Func) {
+    std::tie(Str, Line) = getToken(Line);
+    if (!to_integer(Str, *Size, 16))
+      return false;
+  }
 
   std::tie(Str, Line) = getToken(Line);
-  lldb::addr_t ParamSize;
   if (!to_integer(Str, ParamSize, 16))
-    return llvm::None;
+    return false;
 
-  llvm::StringRef Name = Line.trim();
+  Name = Line.trim();
   if (Name.empty())
-    return llvm::None;
+    return false;
 
-  return PublicRecord(Multiple, Address, ParamSize, Name);
+  return true;
+}
+
+llvm::Optional<FuncRecord> FuncRecord::parse(llvm::StringRef Line) {
+  bool Multiple;
+  lldb::addr_t Address, Size, ParamSize;
+  llvm::StringRef Name;
+
+  if (parsePublicOrFunc(Line, Multiple, Address, &Size, ParamSize, Name))
+    return FuncRecord(Multiple, Address, Size, ParamSize, Name);
+
+  return llvm::None;
+}
+
+bool breakpad::operator==(const FuncRecord &L, const FuncRecord &R) {
+  return L.getMultiple() == R.getMultiple() &&
+         L.getAddress() == R.getAddress() && L.getSize() == R.getSize() &&
+         L.getParamSize() == R.getParamSize() && L.getName() == R.getName();
+}
+llvm::raw_ostream &breakpad::operator<<(llvm::raw_ostream &OS,
+                                        const FuncRecord &R) {
+  return OS << llvm::formatv("FUNC {0}{1:x-} {2:x-} {3:x-} {4}",
+                             R.getMultiple() ? "m " : "", R.getAddress(),
+                             R.getSize(), R.getParamSize(), R.getName());
+}
+
+llvm::Optional<PublicRecord> PublicRecord::parse(llvm::StringRef Line) {
+  bool Multiple;
+  lldb::addr_t Address, ParamSize;
+  llvm::StringRef Name;
+
+  if (parsePublicOrFunc(Line, Multiple, Address, nullptr, ParamSize, Name))
+    return PublicRecord(Multiple, Address, ParamSize, Name);
+
+  return llvm::None;
 }
 
 bool breakpad::operator==(const PublicRecord &L, const PublicRecord &R) {

lldb/source/Plugins/ObjectFile/Breakpad/BreakpadRecords.h

@@ -80,6 +80,31 @@ inline bool operator==(const InfoRecord &L, const InfoRecord &R) {
 }
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const InfoRecord &R);
 
+class FuncRecord : public Record {
+public:
+  static llvm::Optional<FuncRecord> parse(llvm::StringRef Line);
+  FuncRecord(bool Multiple, lldb::addr_t Address, lldb::addr_t Size,
+             lldb::addr_t ParamSize, llvm::StringRef Name)
+      : Record(Module), Multiple(Multiple), Address(Address), Size(Size),
+        ParamSize(ParamSize), Name(Name) {}
+
+  bool getMultiple() const { return Multiple; }
+  lldb::addr_t getAddress() const { return Address; }
+  lldb::addr_t getSize() const { return Size; }
+  lldb::addr_t getParamSize() const { return ParamSize; }
+  llvm::StringRef getName() const { return Name; }
+
+private:
+  bool Multiple;
+  lldb::addr_t Address;
+  lldb::addr_t Size;
+  lldb::addr_t ParamSize;
+  llvm::StringRef Name;
+};
+
+bool operator==(const FuncRecord &L, const FuncRecord &R);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const FuncRecord &R);
+
 class PublicRecord : public Record {
 public:
   static llvm::Optional<PublicRecord> parse(llvm::StringRef Line);

lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp

@@ -26,8 +26,9 @@ namespace {
 class LineIterator {
 public:
   // begin iterator for sections of given type
-  LineIterator(ObjectFile &obj, ConstString section_type)
+  LineIterator(ObjectFile &obj, Record::Kind section_type)
-      : m_obj(&obj), m_section_type(section_type), m_next_section_idx(0) {
+      : m_obj(&obj), m_section_type(toString(section_type)),
+        m_next_section_idx(0) {
     ++*this;
   }
 
@@ -77,7 +78,7 @@ const LineIterator &LineIterator::operator++() {
 }
 
 static llvm::iterator_range<LineIterator> lines(ObjectFile &obj,
-                                                ConstString section_type) {
+                                                Record::Kind section_type) {
   return llvm::make_range(LineIterator(obj, section_type), LineIterator(obj));
 }
 
@@ -180,35 +181,40 @@ void SymbolFileBreakpad::AddSymbols(Symtab &symtab) {
   }
 
   const SectionList &list = *module.GetSectionList();
-  for (llvm::StringRef line : lines(*m_obj_file, ConstString("PUBLIC"))) {
+  llvm::DenseMap<addr_t, Symbol> symbols;
-    auto record = PublicRecord::parse(line);
+  auto add_symbol = [&](addr_t address, llvm::Optional<addr_t> size,
-    if (!record) {
+                        llvm::StringRef name) {
-      LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
+    address += base;
-      continue;
+    SectionSP section_sp = list.FindSectionContainingFileAddress(address);
-    }
-    addr_t file_address = base + record->getAddress();
-
-    SectionSP section_sp = list.FindSectionContainingFileAddress(file_address);
     if (!section_sp) {
       LLDB_LOG(log,
                "Ignoring symbol {0}, whose address ({1}) is outside of the "
                "object file. Mismatched symbol file?",
-               record->getName(), file_address);
+               name, address);
-      continue;
+      return;
     }
+    symbols.try_emplace(
+        address, /*symID*/ 0, Mangled(name, /*is_mangled*/ false),
+        eSymbolTypeCode, /*is_global*/ true, /*is_debug*/ false,
+        /*is_trampoline*/ false, /*is_artificial*/ false,
+        AddressRange(section_sp, address - section_sp->GetFileAddress(),
+                     size.getValueOr(0)),
+        size.hasValue(), /*contains_linker_annotations*/ false, /*flags*/ 0);
+  };
 
-    symtab.AddSymbol(Symbol(
+  for (llvm::StringRef line : lines(*m_obj_file, Record::Func)) {
-        /*symID*/ 0, Mangled(record->getName(), /*is_mangled*/ false),
+    if (auto record = FuncRecord::parse(line))
-        eSymbolTypeCode,
+      add_symbol(record->getAddress(), record->getSize(), record->getName());
-        /*is_global*/ true, /*is_debug*/ false, /*is_trampoline*/ false,
-        /*is_artificial*/ false,
-        AddressRange(section_sp, file_address - section_sp->GetFileAddress(),
-                     0),
-        /*size_is_valid*/ 0, /*contains_linker_annotations*/ false,
-        /*flags*/ 0));
   }
 
-  // TODO: Process FUNC records as well.
+  for (llvm::StringRef line : lines(*m_obj_file, Record::Public)) {
+    if (auto record = PublicRecord::parse(line))
+      add_symbol(record->getAddress(), llvm::None, record->getName());
+    else
+      LLDB_LOG(log, "Failed to parse: {0}. Skipping record.", line);
+  }
 
+  for (auto &KV : symbols)
+    symtab.AddSymbol(std::move(KV.second));
   symtab.CalculateSymbolSizes();
 }

lldb/unittests/ObjectFile/Breakpad/BreakpadRecordsTest.cpp

@@ -50,12 +50,27 @@ TEST(InfoRecord, parse) {
   EXPECT_EQ(llvm::None, InfoRecord::parse("INFO CODE_ID"));
 }
 
+TEST(FuncRecord, parse) {
+  EXPECT_EQ(FuncRecord(true, 0x47, 0x7, 0x8, "foo"),
+            FuncRecord::parse("FUNC m 47 7 8 foo"));
+  EXPECT_EQ(FuncRecord(false, 0x47, 0x7, 0x8, "foo"),
+            FuncRecord::parse("FUNC 47 7 8 foo"));
+
+  EXPECT_EQ(llvm::None, FuncRecord::parse("PUBLIC 47 7 8 foo"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47 7 8"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47 7"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC 47"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC m"));
+  EXPECT_EQ(llvm::None, FuncRecord::parse("FUNC"));
+}
+
 TEST(PublicRecord, parse) {
   EXPECT_EQ(PublicRecord(true, 0x47, 0x8, "foo"),
             PublicRecord::parse("PUBLIC m 47 8 foo"));
   EXPECT_EQ(PublicRecord(false, 0x47, 0x8, "foo"),
             PublicRecord::parse("PUBLIC 47 8 foo"));
 
+  EXPECT_EQ(llvm::None, PublicRecord::parse("FUNC 47 8 foo"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC 47 8"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC 47"));
   EXPECT_EQ(llvm::None, PublicRecord::parse("PUBLIC m"));