[PATCH] D150554: [DWARFLinker][DWARFv5] Add support for .debug_line_str table.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM. I left some nits inline. You also have a few places where you use the names from the DWARF standard (e.g. `directory_entry_format_count`) while in other places you use the name of the variable in the Prologue class (e.g. `SegSelectorSize` instead of `segment_selector_size`). It would be nice to consistently use the ones from the spec to make cross referencing easier.



================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:751
+
+    /// Clone and emit line table for the specified \p Unit.
+    /// Translate directories and file names if necessary.
----------------



================
Comment at: llvm/include/llvm/DWARFLinker/DWARFStreamer.h:233-254
+  void emitLineTablePrologue(const DWARFDebugLine::Prologue &P,
+                             OffsetsStringPool &DebugStrPool,
+                             OffsetsStringPool &DebugLineStrPool);
+  void emitLineTableString(const DWARFDebugLine::Prologue &P,
+                           const DWARFFormValue &String,
+                           OffsetsStringPool &DebugStrPool,
+                           OffsetsStringPool &DebugLineStrPool);
----------------



================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:665
+                                          OffsetsStringPool &DebugLineStrPool) {
+
+  MCSymbol *PrologueStartSym = MC->createTempSymbol();
----------------
spurious newline


================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:672
+  LineSectionSize += 2;
+  if (P.FormParams.Version == 5) {
+    // Address size.
----------------
Any reason you use `P.FormParams.Version` and not `P.getVersion()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150554