Re: [PATCH] Start abbrevs at 1 in DWARF assembler

[Tom de Vries <tdevries@suse.de>]

On 12/5/23 22:23, Tom Tromey wrote:
> I noticed that the DWARF assembler starts abbrevs at 2.
> I think 1 should be preferred.

Agreed.

How about this approach instead:
...
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index f09da0430ab..0813bbbda09 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -958,13 +958,21 @@ namespace eval Dwarf {
  	_handle_macro_at_range $attr_value
      }

+    # Return the next available abbrev number in the current CU's abbrev
+    # table.
+    proc _get_abbrev_num {} {
+	variable _abbrev_num
+	set res $_abbrev_num
+	incr _abbrev_num
+	return $res
+    }
+
      proc _handle_DW_TAG {tag_name {attrs {}} {children {}}} {
  	variable _abbrev_section
-	variable _abbrev_num
  	variable _constants

  	set has_children [expr {[string length $children] > 0}]
-	set my_abbrev [incr _abbrev_num]
+	set my_abbrev [_get_abbrev_num]

  	# We somewhat wastefully emit a new abbrev entry for each tag.
  	# There's no reason for this other than laziness.
...

The nit I see with the patch you propose is that it sets _abbrev_num to 
0, and then claims that 0 is the most recently assigned abbrev number, 
while it was never assigned.

My feeling is that this solution is more intuitive.

Thanks,
- Tom

> ---
>   gdb/testsuite/lib/dwarf.exp | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index f09da0430ab..19c89c048fe 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -564,8 +564,8 @@ namespace eval Dwarf {
>       # for Fission.
>       variable _abbrev_section
>   
> -    # The next available abbrev number in the current CU's abbrev
> -    # table.
> +    # The most recently assigned abbrev number in the current CU's
> +    # abbrev table.
>       variable _abbrev_num
>   
>       # The string table for this assembly.  The key is the string; the
> @@ -1485,7 +1485,7 @@ namespace eval Dwarf {
>   
>   	set cu_num [incr _cu_count]
>   	set my_abbrevs [_compute_label "abbrev${cu_num}_begin"]
> -	set _abbrev_num 1
> +	set _abbrev_num 0
>   
>   	set _cu_label [_compute_label "cu${cu_num}_begin"]
>   	set start_label [_compute_label "cu${cu_num}_start"]
> @@ -1597,7 +1597,7 @@ namespace eval Dwarf {
>   
>   	set cu_num [incr _cu_count]
>   	set my_abbrevs [_compute_label "abbrev${cu_num}_begin"]
> -	set _abbrev_num 1
> +	set _abbrev_num 0
>   
>   	set _cu_label [_compute_label "cu${cu_num}_begin"]
>   	set start_label [_compute_label "cu${cu_num}_start"]