Page MenuHomePhabricator

Don't allow LLDB to try and parse .debug_types
ClosedPublic

Authored by clayborg on Jul 21 2017, 1:31 PM.

Details

Summary

LLDB currently crashes when given .debug_types debug information which is enabled with -fdebug-types when compiling. All complex types are also missing. Seeing as LLDB crashes at worse, or debugs, but provides no type resolution for complex types, we should disable allowing LLDB to debug this until we add support.

Diff Detail

Repository
rL LLVM

Event Timeline

clayborg created this revision.Jul 21 2017, 1:31 PM

This section have been already removed from Dwarf5 so I agree that we shouldn't spend too much time adding support for it. Do you know anybody hitting this issue? Do you know why they decided to use this flag?

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
500–513 ↗(On Diff #107710)

Can we make this disabling logic a bit more fine grained? What I am thinking about is to disable parsing .debug_info and .debug_types but still parse the line table as that shouldn't cause any issue.

501 ↗(On Diff #107710)

I think the name of the flag is "-fdebug-types-section" but it might be platform dependent.

We have been hitting this at Facebook for server apps that statically link the entire world (libc, libc++, libstdc++, all other needed shared libraries) as the debug info is too large unless -fdebug-types-section is used.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
501 ↗(On Diff #107710)

You are correct it is -fdebug-types-section

513 ↗(On Diff #107710)

That patch would be a lot more intrusive as we would still need to be able to parse all compile unit DIEs just so we can get to the line tables. The only way to find the line table for a compile unit is to follow the DW_AT_stmt_list attribute in each top level compile unit DIE. Also, DWARF is useless to us unless we index the DWARF, which means parsing everything in all DIEs. I would like to keep this as simple as possible to just avoid the issue for now. This make it easy to cherry pick this where needed for anyone requiring this patch.

tberghammer accepted this revision.Jul 24 2017, 10:04 AM

Can you file a bug about this issue so we can keep track of it? Also it would be nice to include a small test case in the bug (if you have one) what demonstrates the crash as so far I only managed to see missing type information without an actual LLDB crash.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
513 ↗(On Diff #107710)

You are right.

I thought it is possible to use debug_lines without any information from the debug_info section but it is more complicated then that. In this case it would be almost as complicated to skip the problematic entries as to teach LLDB how to handle them.

This revision is now accepted and ready to land.Jul 24 2017, 10:04 AM

I believe a crash looks like:

#include <stdio.h>

int main(int argc, const char **argv) {
  typedef enum FooTag {
    Bar,
    Baz
  } Foo;
  Foo foo = Bar;
  printf("foo = %i\n", foo);
  return 0; // Break here and "frame variable"
}

The enum gets put into a type unit, and then we get an assertion when trying to get the byte size for an integer whose byte size is zero. -fdebug-types-section doesn't work on Mac as the flag is just ignored. Also, can we just assume "-fdebug-types-section" is available for any compiler that we run the LLDB test suite with? I am guessing that isn't the case so we should limit any test case to GCC or clang.

Can you compile the above code snippet and see if it crashes on linux?

The enum might need to be scoped outside the function or in a header file...

This revision was automatically updated to reflect the committed changes.

This section have been already removed from Dwarf5 so I agree that we shouldn't spend too much time adding support for it.

Compilers wanting to use type units and DWARF 4 should be emitting them in the .debug_types section. DWARF 5 kept the concept but moved them back into the .debug_info section. Supporting type units in general seems like a good idea, and the only question is where they live and what the exact details of the header look like.

This section have been already removed from Dwarf5 so I agree that we shouldn't spend too much time adding support for it.

Compilers wanting to use type units and DWARF 4 should be emitting them in the .debug_types section. DWARF 5 kept the concept but moved them back into the .debug_info section. Supporting type units in general seems like a good idea, and the only question is where they live and what the exact details of the header look like.

I have a patch that adds support for .debug_types seamlessly with DWARF5:

https://reviews.llvm.org/D32167

I haven't gotten the time to complete it and get all the buildbots clean, but it is in the queue.

I tried to compile the code snippet you mentioned but it doesn't put anything into the debug_types section. If I move the enum definition outside of the class then it produces debug_type section but LLDB fails back gracefully without any crash or error message. I also tried a few other code snippets and the debugging experience usually wasn't great (types of local variables were messed up) but I haven't manage to get LLDB crashing.