[PATCH 2/3] [binutils/readelf] Add fixme in display_debug_str_offsets

[Alan Modra]

On Wed, May 15, 2024 at 10:06:39AM +0200, Tom de Vries wrote:
> On 5/15/24 09:37, Alan Modra wrote:
> > On Tue, May 14, 2024 at 12:55:21PM +0200, Tom de Vries wrote:
> > > While reading display_debug_str_offsets, I realized that the v4 split dwarf
> > > handling bit doesn't assign entry_length, so it can be either 4 (32-bit dwarf)
> > > or 8 (64-bit dwarf).  However, I think the cases for which it'll be 8 are
> > > rare and odd enough to conclude that in practise we default to 32-bit
> > > dwarf.
> > > 
> > > As mentioned in the fission documentation [1], whether a .debug_str_offsets
> > > section is 32-bit or 64-bit dwarf depends on the compilation unit header in
> > > the .debug_info.dwo section, which is currently not inspected.
> > > 
> > > For now:
> > > - make an explicit choice for 32-bit dwarf by assiging 4 to entry_length, and
> > > - add a fixme comment.
> > > 
> > > [1] https://gcc.gnu.org/wiki/DebugFission
> > > ---
> > >   binutils/dwarf.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/binutils/dwarf.c b/binutils/dwarf.c
> > > index 8125c64e1b6..f49989dc536 100644
> > > --- a/binutils/dwarf.c
> > > +++ b/binutils/dwarf.c
> > > @@ -7971,6 +7971,14 @@ display_debug_str_offsets (struct dwarf_section *section,
> > >   	  entries_end = end;
> > >   	  debug_str_offsets_hdr_len = 0;
> > > +	  /* FIXME: We assume 32-bit dwarf here.  According to
> > > +	     https://gcc.gnu.org/wiki/DebugFission:
> > > +	     The size of each entry is 4 bytes for DWARF-32 compilation units,
> > > +	     or 8 bytes for DWARF-64 compilation units, as determined by the
> > > +	     unit_length field of the compilation unit header in the
> > > +	     .debug_info.dwo section.  */
> > > +	  entry_length = 4;
> > > +
> > >   	  printf (_("    Length: %#" PRIx64 "\n"), length);
> > >   	  printf (_("       Index   Offset [String]\n"));
> > >   	}
> > > -- 
> > > 2.35.3
> > 
> > I don't see the need for this patch.  If length is zero we already
> > have entry_length of four.
> > 
> 
> This follow-up demonstrator patch:
> - adds an assert asserting that entry_length is four, and
> - modifies the test-case to trigger it.
> 
> Thanks,
> - Tom
> 
> diff --git a/binutils/dwarf.c b/binutils/dwarf.c
> index cdf73a7bc50..586c87c3afd 100644
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -7985,6 +7985,7 @@ display_debug_str_offsets
>  	     or 8 bytes for DWARF-64 compilation units, as determined by the
>  	     unit_length field of the compilation unit header in the
>  	     .debug_info.dwo section.  */
> +	  assert (entry_length == 4);
>  	  entry_length = 4;
> 
>  	  printf (_("    Length: %#" PRIx64 "\n"), length);
> diff --git a/binutils/testsuite/binutils-all/readelf-debug-str-offsets-dw4.s
> b/binutils/testsuite/binutils-all/read
> elf-debug-str-offsets-dw4.s
> index 1f16f4571e3..45f79be75f2 100644
> --- a/binutils/testsuite/binutils-all/readelf-debug-str-offsets-dw4.s
> +++ b/binutils/testsuite/binutils-all/readelf-debug-str-offsets-dw4.s
> @@ -2,8 +2,8 @@
>  	.asciz	"FIRST"
>  	.asciz	"SECOND"
>  	.section	.debug_str_offsets.dwo,"MS",%progbits,1
> +	.4byte 0xffffffff
>  	.4byte 0
> -	.4byte 6
>  	.section        .debug_macro.dwo,"e",%progbits
>  	.2byte  0x4     /* DWARF macro version number.  */
>  	.byte   0x0     /* Flags: 32-bit dwarf.  */

Sorry, that doesn't look like a realistic testcase to me.  An offset
of 0xffffffff in 32-bit dwarf?

-- 
Alan Modra