[PATCH] D67723: [DebugInfo] Add option to disable inline line tables.

Mon Oct 28 16:42:28 PDT 2019

On Thu, Oct 24, 2019 at 3:24 PM Adrian Prantl via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

>
>
> On Oct 24, 2019, at 3:02 PM, David Blaikie via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
> dblaikie added a comment.
>
> In D67723#1720509 <https://reviews.llvm.org/D67723#1720509>, @aprantl
> wrote:
>
> In D67723#1720353 <https://reviews.llvm.org/D67723#1720353>, @rnk wrote:
>
> In D67723#1717468 <https://reviews.llvm.org/D67723#1717468>, @aprantl
> wrote:
>
> I agree that it would make sense to have a
> `-ginline-info-threshold=<#insns>` or `-gno-small-inline-functions` with a
> hardcoded threshold to implement the feature Paul described, and this patch
> seems to be a step in that direction, with the threshold being hardcoded to
> 0.
>
>
>
> OK. :)
>
> We are motivated by one tool in particular at the moment, but if we're
> going to take the time to add a knob, we might as well make it work for
> DWARF.
>
>
> Here you got me confused: When I read "we might as well make it work for
> DWARF", I read that as "we should emit the inlined instructions with line 0
> under a DWARF debugger tuning". But that reading seems to to contradict
> your next sentence:
>
> If the user cares enough to find this flag, it seems more user friendly to
> make it behave the same rather than making it format-dependent.
>
>
> Can you clarify?
>
>
> If we use line zero for DWARF, gdb will not behave in the way documented
> by the function attribute in LangRef. I was the one who suggested the
> wording there, so maybe we could come up with new wording that describes
> what the user should expect in the debugger when using line zero. However,
> given the behavior I show below, I have a hard time imagining the use case
> for it.
>
>
>
> I didn't realize that GDB also had problems; I thought that this was a
> problem that only affected Windows debuggers.
>
>
>
> I don't think the behavior Reid described would be a "problem" - it seems
> to me like the only behavior the debugger could provide if those
> instructions are attributed to line zero.
>
>
>
> I applied the version of this patch that uses getMergedLocation, compiled
> this program, and ran it under gdb:
>
>  volatile int x;
>  static inline void foo() {
>    ++x;
>    *(volatile int*)0 = 42; // crash
>    ++x;
>  }
>  int main() {
>    ++x;  // line 8
>    foo();  // line 9
>    ++x;
>    return x;
>  }
>
>
> If we apply line zero, the debugger stops on line 8:
>
>  Program received signal SIGSEGV, Segmentation fault.
>  0x000000000040111e in main () at t.cpp:8
>  8         ++x;
>  (gdb) bt
>  #0  0x000000000040111e in main () at t.cpp:8
>
>
> The inline frame is gone, as expected for this flag, but the current
> location does not reflect the site of the call to `foo`. So, if we want it
> to behave as documented, we have to put the call site location on some
> instructions.
>
> Alternatively, if I arrange things like this, the crash is attributed to
> line `return x`, which is completely unrelated to the inline call site:
>
>  static inline void foo() {
>    ++x;
>    if (x) {
>      *(volatile int*)0 = 42; // crash
>      __builtin_unreachable();
>    }
>    ++x;
>  }
>
>
> This means that if line zero is used, the source location shown in the
> debugger becomes sensitive to code layout, which is arbitrary.
>
> These experiments are convincing me that, in general, line zero isn't that
> helpful for DWARF consumers. If the goal is to get smooth stepping, we may
> want to refocus on getting reliable is_stmt bits in the line table.
>
>
> The Swift compiler is far more aggressive in using line 0 than Clang, and
> consequently LLDB is much better at handling line 0 than even GDB, and that
> can skew my perception :-)
>
>
> What behavior does LLDB have in the example Reid gave?
>
>
> I short-circuited a little by marking the function always_inline and
> putting a .loc 1 0 0  before the inlined instructions, so I hope that
> didn't butcher the example, but I didn't want to wait for clang to compile.
> LLDB says
>
> * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS
> (code=1, address=0x0)
>     frame #0: 0x0000000100000f9f a.out`main at reid.c:0 [opt]
>

I meant more as a comparison with the stepping (rather than backtracing)
behavior that Reid mentioned from GDB.

> If the intended behavior is to show the crash at the call site like
> LangRef in the patch suggest then line 0 will certainly not achieve that. I
> implicitly assumed the wording in LangRef would follow the implementation
> if we switched to line 0.
>

Right - I think there's some reasonable discussion about what would be
suitable/useful behavior - and line zero isn't necessarily an experience a
user would want in this situation. (if there's no user for the feature, why
build it - if we have a quality/space tradeoff that seems useful to
CodeView users & we know inlining info is expensive in DWARF too (maybe not
as expensive as CodeView, I'm not sure) & so DWARF users /might/ want the
same tradeoff, seems like it might be worth providing the same
feature/behavior in both)

Put another way: I don't know/don't think Reid's users would want the
feature using line zero even if CV had support for something equivalent to
DWARF's line zero. The "attribute it to the call site" behavior is more
meaningful/useful than line zero to a user.

- Dave

>
> -- adrian
>
>
> Give how popular GDB is, I don't want to intentionally break compatibility
> with it, so I think this patch is okay. If we wanted we can put an
> if-debugger-tuning-is-LLDB-getMergedLocation condition in. Otherwise
> documenting that this is necessary for compatibility with popular
> debuggers, seems fine to me, too.
>
>
>
>
>
> Repository:
>  rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D67723/new/
>
> https://reviews.llvm.org/D67723
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191028/fafab92c/attachment.html>