[PATCH] D108261: [DebugInfo] Fix end_sequence of debug_line in LTO Object

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Looks roughly right - one trailing question I wouldn't mind knowing the answer to (might merit a comment so it can be cleaned up later) - and also, maybe worth adding an assembly test case. Something like this:

  .text
  .file   1 "small.c"
  .loc    1 1 0
  nop
  .section .text.2
  .loc    1 2 0
  nop
  .text
  nop

Showing that line table locations in assembly do flow on to the next chunk of a section even if there's an intermediate section switch. So unlike with the DwarfDebug case, which can end the line table after the function/whenever switching compilation units - the assembly mode might have any number of outstanding sections that need to be terminated at the end. (I guess in theory we could optimize the IR/DwarfDebug case slightly by keeping some of these open - but I don't think it likely in practice & seems cleaner with what this patch does now)



================
Comment at: llvm/include/llvm/MC/MCDwarf.h:221
+  void addEndEntry(MCSymbol *EndLabel) {
+    if (PrevSec) {
+      auto *LastEntry = &MCLineDivisions[PrevSec].back();
----------------
kyulee wrote:
> kyulee wrote:
> > dblaikie wrote:
> > > When does this case come up? I think all of this would only happen when PrevCU was non-null/had been populated with some content first, right?
> > Correct. I changed it to an assert.
> It turned out that there is the case where the line entry is empty in PrevCU. So I restored it with this check.
When does that case arise? (maybe when a function is zero-length/has no instructions? That's something I/we would like to fix at some point, FWIW)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:497
           // symbols are (with entries in .debug_addr).
-          // For now, since we only ever use index 0, this should work as-is.       
+          // For now, since we only ever use index 0, this should work as-is.
           addUInt(*Loc, dwarf::DW_FORM_data4, FrameBase.Location.WasmLoc.Index);
----------------
Unrelated change - could commit this separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108261