Page MenuHomePhabricator

SymbolFileBreakpad: Add line table support
Changes PlannedPublic

Authored by labath on Fri, Jan 11, 5:08 AM.

Details

Summary

This patch teaches SymbolFileBreakpad to parse the line information in
breakpad files and present it to lldb.

The trickiest question here was what kind of "compile units" to present
to lldb, as there really isn't enough information in breakpad files to
correctly reconstruct those.

The two options I considered were:

  • have the entire file be one compile unit
  • have one compile unit for each FILE record

The drawbacks of the first approach are that the compile unit created
this way will be huge, and there isn't a good way to name it (I decided
to name it after the object file).

The second approach will create mostly correct compile units for cpp
files, but it will still be wrong for headers. However, the biggest
drawback here seemed to be the fact that this can cause a compile unit
to change mid-function (for example when a function from another file is
inlined or another file is #included into a function). While I don't
know of any specific thing that would break in this case, it does sound
like a thing that we should avoid.

So, in the end, I chose the first approach, because it results in
simpler code, and having one compile unit doesn't seem so bad,
particularly as the second approach wouldn't result in correct compile
units either.

Event Timeline

labath created this revision.Fri, Jan 11, 5:08 AM
labath marked an inline comment as done.Fri, Jan 11, 5:13 AM
labath added inline comments.
source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
150–151

Note that here i set file=column=line=0 for the terminal entry, which isn't consistent with the dwarf plugin for instance (it puts there whatever falls out of the state automaton, which most likely means the values from the previous entry). AFAICT, this shouldn't be a problem, because the terminal entry is there to just determine the range of the last real entry.

So LLDB treats compile units special depending on the inline strategy. Because of this, I outlined some examples of how and why we should create a compile unit per "FUNC" token. Let me know if anything above was unclear

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

I would vote to make 1 compile unit for each "FUNC". The hard part will be to select the right source file for each function. I would just start by selecting the first line entry for the function. Compile units are expected to give a list of support files, and in this case, you would make a set of files that are in the line entries only. So if you had:

MODULE Linux x86_64 761550E08086333960A9074A9CE2895C0 a.out
INFO CODE_ID E05015768680393360A9074A9CE2895C
FILE 0 /tmp/a.c
FILE 1 /tmp/b.c
FILE 2 /usr/include/foo.h
FUNC b0 10 0 func1
b0 1 1 0
b1 1 2 0
b2 1 2 1
b4 1 3 1
FUNC b0 10 0 func2
b0 1 1 1
b1 1 2 1
b2 1 2 2

We would have a compile unit for "func1" with a cu for "/tmp/a.c" and with support files:
support_file[0] = "/tmp/a.c"
support_file[1] = "/usr/include/foo.h"

We would have a compile unit for "func2" with a cu for "/tmp/b.c" and with support files:

support_file[0] = "/tmp/b.c"
support_file[1] = "/usr/include/foo.h"

The main reason for make individual compile units, is LLDB treats a compile unit specially when settings breakpoints depending on if we ask for inline functions to be set. If we set the following setting:

(lldb) settings set target.inline-breakpoint-strategy never

Then we will check the name of the compile unit to ensure it matches. So if we did:

(lldb) b a.c:12

This would only work if the actual lldb_private::CompileUnit has a FileSpec that matches "a.c".

122

return the count of the number of "FUNC" objects

123–125

parse a compile unit per function. We might want to cache all "FILE" entries in a list inside the SymbolFileBreakpad so we can easily pull out the FileSpecs when creating each compile unit.

Also, each compile unit's ID can be the line number in the breakpad file to the "FUNC" entry. This allows easy access to each "FUNC" entry in the breakpad file when we are asked to parse more information about it (get compile unit support files, or any parsing of info for a compile unit.

158–179

It would be nice to put this parsing code into the lldb_private::breakpad::Line" class I talked about in the other patch?

It would be great if this code looked like:

lldb_private::breakpad::Line bp_line(line);
switch (bp_line.GetToken()) {
  case Token::Func:
    // Create the compile unit and store into cu_sp
    cu_sp = bp_line.CreateCompileUnit();
    break;
  case Token::Line: {
    addr_t address;
    size_t size;
    uint32_t line_num, file_num;
    if (bp_line.ParseLineEntry(address, size, line_num, file_num)) {
      // Discontiguous entries. Finish off the previous sequence and reset.
      if (next_addr && *next_addr != address)
        finish_sequence();
      line_table_up->AppendLineEntryToSequence(
          line_seq_up.get(), address, line_num, 0, file_num, true, false, false, false,  false);
    }
199

This function will need to populate support_files for a given FUNC as mentioned above

234

We need to search each compile unit to see which compile unit contains the address now.

source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
99

Remove whitespace only change

labath planned changes to this revision.Thu, Jan 17, 5:08 AM

I think I understand what you mean. I'll try to refactor this to create a compile unit for each function.