labath created this revision. labath added reviewers: clayborg, JDevlieghere, aprantl.
r362103 exposed a bug, where we could read incorrect data if a skeleton unit contained more than the single unit DIE. Clang emits these kinds of units with -fsplit-dwarf-inlining (which is also the default). Changing lldb to handle these DIEs is nontrivial, as we'd have to change the UID encoding logic to be able to reference these DIEs, and fix up various places which are assuming that all DIEs come from the separate compile unit. However, it turns out this is not necessary, as the DWO unit contains all the information that the skeleton unit does. So, this patch just deletes any extra DIEs which are present in the skeleton unit, and enforces the invariant that the rest of the code is already operating under. This patch fixes a couple of existing tests, but I've also included a simpler test which does not depend on execution of binaries, and would have helped us in catching this sooner. https://reviews.llvm.org/D62852 Files: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -240,6 +240,15 @@ m_die_array.shrink_to_fit(); if (m_dwo_symbol_file) { + // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton compile + // units. We are not able to access these DIE *and* the dwo file + // simultaneously. We also don't need to do that as the dwo file will + // contain a superset of information. So, we just delete these extra DIEs + // (if any) and reclaim some space. + m_die_array.resize(1); + m_die_array.shrink_to_fit(); + m_die_array[0].SetHasChildren(false); + DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit(); dwo_cu->ExtractDIEsIfNeeded(); } Index: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp =================================================================== --- /dev/null +++ lit/SymbolFile/DWARF/split-dwarf-inlining.cpp @@ -0,0 +1,8 @@ +// RUN: %clangxx -target x86_64-pc-linux -gsplit-dwarf -fsplit-dwarf-inlining \ +// RUN: -c %s -o %t +// RUN: %lldb %t -o "breakpoint set -n foo" -b | FileCheck %s + +// CHECK: Breakpoint 1: 2 locations + +__attribute__((always_inline)) int foo(int x) { return x; } +int bar(int x) { return foo(x); }
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -240,6 +240,15 @@ m_die_array.shrink_to_fit(); if (m_dwo_symbol_file) { + // With -fsplit-dwarf-inlining, clang will emit non-empty skeleton compile + // units. We are not able to access these DIE *and* the dwo file + // simultaneously. We also don't need to do that as the dwo file will + // contain a superset of information. So, we just delete these extra DIEs + // (if any) and reclaim some space. + m_die_array.resize(1); + m_die_array.shrink_to_fit(); + m_die_array[0].SetHasChildren(false); + DWARFUnit *dwo_cu = m_dwo_symbol_file->GetCompileUnit(); dwo_cu->ExtractDIEsIfNeeded(); } Index: lit/SymbolFile/DWARF/split-dwarf-inlining.cpp =================================================================== --- /dev/null +++ lit/SymbolFile/DWARF/split-dwarf-inlining.cpp @@ -0,0 +1,8 @@ +// RUN: %clangxx -target x86_64-pc-linux -gsplit-dwarf -fsplit-dwarf-inlining \ +// RUN: -c %s -o %t +// RUN: %lldb %t -o "breakpoint set -n foo" -b | FileCheck %s + +// CHECK: Breakpoint 1: 2 locations + +__attribute__((always_inline)) int foo(int x) { return x; } +int bar(int x) { return foo(x); }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits