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? > 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@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
