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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:171-173
+      // If it is the last entry, don't emit another end entry after the loop.
+      if (&LineEntries.back() == &LineEntry)
+        return;
----------------
When does this occur? I guess when the last function with debug info in this section is followed by a function in another section (with or without debug info) or a nodebug function?

That does seem a bit subtle and like it'd be nicer if this API's invariant was just that all callers terminated their own lists. (this would involve fixing up the GenDwarfForAssembly codepath to do that too, I'd guess)

For DwarfDebug/etc I guess this would be addressed by calling terminateLineTableForPrevCU (or perhaps resetPrevCU) in finishModule or whatever it's called?


================
Comment at: llvm/lib/MC/MCDwarf.cpp:226
   // Generate DWARF line end entry.
   MCOS->emitDwarfLineEndEntry(Section, LastLabel);
 }
----------------
kyulee wrote:
> dblaikie wrote:
> > Would be nice if we could eliminate the need for this case - since any time this gets used it risks being because a line table was allowed to "flow off the end" further than it should.
> > 
> > What would it be like if we handled the line table similar to the way ranges are handled? Always terminated at the end of each function, but then extended if the next function happens to start immediately after the last entry ended?
> Yeah. That sounds great, but it seems similar to what I initially tried, which tracked the end label of function as functions were added.
> But I wonder whether there is a case where the end label of the prior function is not matched with the start label of the next function due to some alignment, etc? In that case, do we want to terminate the line table for the prior function because of mismatch? I guess that seems an overkill. 
> 
Yeah, though I was thinking more driven by the DwarfDebug code so it's not tracked in two different places that might diverge/reduce the code/logic needed to handle these cases.

I think alignment doesn't break this strategy (it doesn't seem to break it for the ranges data using this approach) - though the alignment does come between the start of one function and the beginning of the next function - the code in DwarfDebug/DwarfCompileUnit/etc extends the CU ranges to cover that & hopefully the line table could be powered by the same logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108261