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

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D67723#1708671 <https://reviews.llvm.org/D67723#1708671>, @aprantl wrote:

> Who is "we" in this context? The CodeView backend?


Yes, the CodeView backend, sorry for the ambiguity.

> As far as DWARF is concerned (and LLVM mostly inherits the DWARF semantics) line 0 is well-defined and means compiler-generated code or otherwise no unambiguous source location. DWARF-based debuggers know to skip over instructions with line 0.
> 
> Is the problem that CodeView doesn't have this concept, or does the Windows debugger no know how to deal with it (or both)?

Visual Studio in particular has been reported to have problems with line zero. It seems to treat it as some kind of error condition (no line available), and kicks the user over to the assembly view.

> I'm feeling rather strongly that that LLVM should not be emitting wrong debug info to work around bugs in a debugger. I understand that sometimes this isn't possible because we don't control the consumers. The correct thing to do here is to guard the workaround by a debugger tuning flag. For DWARF, we do want line 0 here.

I don't think we want to emit line zero here. The use case for this flag is to allow the user to ask the compiler to attribute code from inlined call sites to the call site itself. Maybe the user doesn't want to see the details of push_back.

I actually went ahead and experimented with how gdb handles line zero. I compiled the following program like so:

  $ cat t.cpp
  volatile int x;
  static inline void foo() {
    ++x;
    ++x;
  }
  int main() {
    ++x;
    foo();
    ++x;
    foo();
    ++x;
    return x;
  }
  $ clang -g -O2 t.cpp -S -emit-llvm -o t.ll
  $ sed -e 's/DILocation.line:.*column:.*, scope\(.*inlinedAt: .*\))/DILocation(line: 0, column: 0, scope\1)/' -i t.ll
  $ clang -g t.ll -o t.exe
  $ gdb --args t.exe
  ...

When I step through main with `s` in gdb, it stops on the `++x` lines, and skips the `foo()` lines completely. I don't think that's the desired behavior, the desired behavior is to treat the body of `foo` as a single line.

To give more context, back in 2013, @probinson asked if we should add a similar feature here:
http://lists.llvm.org/pipermail/cfe-dev/2013-November/033765.html
This was around the time that inlinedAt locations were being sorted out, I think. His proposed name for this flag was -gno-inlined-scopes. I believe nothing ever came of that discussion, and we continued on our way until today in 2019, when one of our users requested a similar feature. I vaguely recalled the discussion from 2013, but I had forgotten the details, so I figured that it might be a generally useful feature that others would appreciate. So, I suggested that @akhuang add a real flag for it, and that it should work for DWARF and CodeView. That's part of why I figured it would be good to implement it in the inliner, so we don't have to do the work twice.

Given the behavior of gdb shown above, I don't think either set of users, Chromium developers or Sony licensees, have a use case for a flag that applies line zero to inlined functions. I don't think that's what they are asking for. Paul outlined what users actually asked for back then here:
http://lists.llvm.org/pipermail/cfe-dev/2013-November/033782.html

I think users are asking for a flag that attributes the code that the inliner clones to the call site. And, I can't see any reason not to give it to them. Does that seem reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67723