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

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-str-offsets-invalid-5.s:13
+# INVALIDSECTIONLENGTH: .debug_str_offsets contents:
+# INVALIDSECTIONLENGTH: 0x00000000: Gap, length = 1
----------------
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).


================
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.
----------------
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 :)


================
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
----------------
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.


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

https://reviews.llvm.org/D78555