[Lldb-commits] [PATCH] D81334: 2/2: [lldb] Fix DW_TAG_GNU_call_site-DW_AT_low_pc as produced by GCC

Mon Jun 8 13:18:35 PDT 2020

jankratochvil marked 4 inline comments as done.
jankratochvil added inline comments.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h:113

-  size_t GetAttributes(DWARFAttributes &attributes, uint32_t depth = 0) const;
+  size_t GetAttributes(DWARFAttributes &attributes, int32_t depth = 0) const;

----------------
labath wrote:
> Using -1 to prevent recursion is pretty unobvious. I think it would be better to add a `bool recurse = true` argument between `attributes` and `depth`. In fact, I'd consider even deleting the `depth` argument -- it's an internal detail that noone except `DWARFDebugInfoEntry` should be using and the single usage there can be easily changed to `spec_die.GetDIE()->GetAttributes(spec_die.GetUnit(), attributes, recurse, depth+1)`
I had this idea. :-) The problem is `bool recurse` is type-compatible with `uint32_t depth` which leads to uncaught bugs. Given that non-recursive call makes no sense with `depth>0` I find the special value `depth = -1` to be suitable for non-recursive calls.
Anyway implemented the `recurse` parameter in a safe way if it is worth it to separate it.

================
Comment at: lldb/test/Shell/SymbolFile/DWARF/DW_TAG_GNU_call_site-DW_AT_low_pc.s:14
+# RUN: %clang_host -o %t %s
+# RUN: %lldb %t -o 'b 6' -o r -o 'p p' -o exit | FileCheck %s
+
----------------
labath wrote:
> Maybe place an int3 at the place where you want this to stop? Then you can delete all of the line table directives and the break location will be more obvious...
Done, nice.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81334/new/

https://reviews.llvm.org/D81334