[PATCH] D78555: [DebugInfo][DWARF64] Fix dumping pre-standard .debug_str_offsets.dwo sections.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 4 inline comments as done.
ikudrin added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-str-offsets-invalid-5.s:13
+# INVALIDSECTIONLENGTH: .debug_str_offsets contents:
+# INVALIDSECTIONLENGTH: 0x00000000: Gap, length = 1
----------------
dblaikie wrote:
> wolfgangp wrote:
> > ikudrin wrote:
> > > wolfgangp wrote:
> > > > This isn't particularly important but it would be nice to emit a more precise error message on a degenerate .debug_str_offsets section like this.
> > > Currently, we report the irregularities found in the section, in some other way, but still. Did you mean some specific case which should be reported better/differently?
> > Just this particular case, where the section isn't big enough to accommodate even one single entry, let alone a contribution header. I thought it might be a bit more user-friendly to say something like "the section is too small to contain any meaningful information" instead of reporting a gap, which might be confusing.
> Yeah, I probably wouldn't encourage special-casing it - I'd slightly prefer the consistency. But what's binutils objdump do with that sort of situation (I know it prints "gaps" when dumping debug_loc - so might be worth seeing what it prints when there's only a gap/nothing else/especially short gap).
Well, I am inclining to avoid special cases like that. They are already covered with the existing diagnostics.

As for `objdump`, it seems like it does not print a separate report for the string offsets section.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-str-offsets-v4-dwarf64-dwo.s:1-3
+## This tests dumping a .debug_str_offsets.dwo section which is referenced by
+## DWARF64 pre-v5 units and dumping attributes in such units which use the
+## DW_FORM_strx form.
----------------
dblaikie wrote:
> Honestly I'm not sure this situation is worth supporting - DWARFv4, non-standard extension (split DWARF), DWARF64. Might be nice to just go forward - supporting v5, standard split DWARF, DWARF64 - but if the v4 non-standard extension in DWARF64 is something you really want to support, so be it :)
This is mostly for completeness and simplifying the code at the same time. The DWARF64 v5 cases seem to be already supported.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-str-offsets-v4-dwarf64-dwo.s:21-30
+# CHECK:      .debug_str.dwo contents:
+# CHECK-NEXT: 0x00000000: "foo"
+# CHECK-NEXT: 0x00000004: "bar"
+# CHECK-NEXT: 0x00000008: "baz"
+
+# CHECK:      .debug_str_offsets.dwo contents:
+# CHECK-NEXT: 0x00000000: Contribution size = 24, Format = DWARF64, Version = 4
----------------
dblaikie wrote:
> Probably worth using some strings of different lengths (especially not all 4 or 8 bytes long)  to make it clearer what's the string offset versus the byte offset of the record, etc.
Makes sense. Thanks!


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

https://reviews.llvm.org/D78555