[PATCH] D42707: [DWARFv5] Emit .debug_line_str (in a non-DWO file)

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 30 16:43:44 PST 2018


probinson added inline comments.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:78
+  /// Indicate whether we're emitting a .debug_line_str section.
+  operator bool() { return LineStrLabel != nullptr; }
+};
----------------
dblaikie wrote:
> Any op bool should be marked explicit - otherwise there's all manner of implicit conversions that can get weird.
Got it.
Would you rather have it be a normal predicate method?  I went with operator bool mainly for compactness.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:482
   if (LineTableVersion >= 5)
-    emitV5FileDirTables(MCOS);
+    emitV5FileDirTables(MCOS, LineStr);
   else
----------------
dblaikie wrote:
> I'm not quite sure - is there a reason that emitV5FileDirTables handle the whole MCDwarfLineStr functionality internally without having to pass in an MCDwarfLineStr object? (emitV5FileDirTables could create the MCDwarfLineStr locally, potentially & emit it immediately?)?
When there are multiple CUs (e.g. LTO) we want the MCDwarfLineStr instance to span emitting all of them.  Otherwise we can get multiple instances of the same path in the same .debug_line_str section, in a single .o file.
I could have put MCDwarfLineStr in the MCContext, but this way we have better data hiding.
Also we need a parameter of some kind anyway, to distinguish the DWO and non-DWO cases.


================
Comment at: llvm/test/MC/ELF/debug-md5.s:20-25
+# CHECK: .debug_line_str contents:
+# CHECK-NEXT: 0x00000000: ""
+# CHECK-NEXT: 0x00000001: "dir1"
+# CHECK-NEXT: 0x00000006: "dir2"
+# CHECK-NEXT: 0x0000000b: "foo"
+# CHECK-NEXT: 0x0000000f: "bar"
----------------
dblaikie wrote:
> Any way to test that the line table is actually using the indexes into the debug_line_str section?
Maybe, when printing the directory/file names we could print the section+index when they're indirect.  I'll look into that.


Repository:
  rL LLVM

https://reviews.llvm.org/D42707





More information about the llvm-commits mailing list