Re: [PATCH 2/6] gdb/testsuite/dwarf: convert _section proc to use parse_options

Simon Marchi
2025-11-19T20:40:52Z

On 2025-11-19 10:53, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
>
> I meant to send this last week but forgot.
>
> Simon> - proc _section {name {flags ""} {type ""}} {
> Simon> - if {$name == ".debug_str"} {
> Simon> - # Hard-code this because it's always desirable.
> Simon> - _emit " .section $name, \"MS\", %progbits, 1"
> Simon> - } elseif {$flags == "" && $type == ""} {
> Simon> - _emit " .section $name"
> Simon> - } elseif {$type == ""} {
> Simon> - _emit " .section $name, \"$flags\""
> Simon> - } else {
> Simon> - _emit " .section $name, \"$flags\", %$type"
>
> This code ignores flags and type if name is .debug_str, but the
> replacement code:
>
> Simon> + if { $name == ".debug_str" } {
> Simon> + # Hard-code this because it's always desirable.
> Simon> + append directive ", \"MS\", %progbits, 1"
> Simon> + }
> Simon> +
> Simon> + if { $flags != "" } {
> Simon> + append directive ",\"$flags\""
> Simon> + }
> Simon> +
> Simon> + if { $type != "" } {
> Simon> + # Can't specify a type without flags.
> Simon> + if { $flags == "" } {
> Simon> + append directive ", \"\""
> Simon> + }
> Simon> +
> Simon> + append directive ", %$type"
> Simon> + }
>
> ... does not.
>
> I think the old code was clearer in this regard. I didn't read forward
> in the series to see if this restructuring helps newer code though.
> Anyway this change should be addressed.

It's true that if one was to call the proc with $name == ".debug_str"
and some flags, the resulting assembler directive would be malformed. I
can change it to

if { $name == ".debug_str" } {
# Hard-code this because it's always desirable.
_emit " .section .debug_str, \"MS\", %progbits, 1"
return
}

I think that would make it clear again that this is a special case,
disconnected from the rest.

Or something cleaner would be to remove this bit from this proc, and
add a _section_debug_str proc that would call _section with the right
params (might need a _defer_output_debug_str as well).

Simon