Start abbrevs at 1 in DWARF assembler
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
I noticed that the DWARF assembler starts abbrevs at 2.
I think 1 should be preferred.
---
gdb/testsuite/lib/dwarf.exp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
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"]
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>> I noticed that the DWARF assembler starts abbrevs at 2.
>> I think 1 should be preferred.
Tom> Agreed.
Tom> How about this approach instead:
It's fine by me, but FWIW I have also been toying with fixing the buglet
that abbrevs aren't cached, because I happened to notice that
mega-enum.exp makes an executable with 65000+ abbrevs where only 5 or so
are actually needed. This patch would obsolete this counter entirely.
Tom
On 12/6/23 16:41, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
>>> I noticed that the DWARF assembler starts abbrevs at 2.
>>> I think 1 should be preferred.
>
> Tom> Agreed.
>
> Tom> How about this approach instead:
>
> It's fine by me,
Pushed as attached.
> but FWIW I have also been toying with fixing the buglet
> that abbrevs aren't cached, because I happened to notice that
> mega-enum.exp makes an executable with 65000+ abbrevs where only 5 or so
> are actually needed. This patch would obsolete this counter entirely.
>
Ack.
Thanks,
- Tom
@@ -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"]