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

[Tom de Vries]

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.  */