Re: [PATCH v2 1/2] gdb/testsuite: add .debug_frame support in DWARF assembler

[Tom Tromey <tom@tromey.com>]

>>>>> "Simon" == simon marchi <simon.marchi@polymtl.ca> writes:

Simon> Add support to the DWARF assembler for generating .debug_frame sections.
Simon> My initial use case is to reproduce a crash happening when encountering
Simon> an empty FDE, but I suppose that other use cases will pop up in the
Simon> future.

Thanks for doing this.  I have one little note.

Simon> +		# Create procs for DW_CFA_* instructions, used in
Simon> +		# .debug_frame CIE/FDE bodies.
Simon> +
Simon> +		# DW_CFA_advance_loc, DW_CFA_offset and
Simon> +		# DW_CFA_restore encode the operand in the low 6
Simon> +		# bits of the opcode byte.  They need special
Simon> +		# handling.
Simon> +		switch -exact -- $name {
Simon> +		    DW_CFA_advance_loc {
Simon> +			proc DW_CFA_advance_loc {delta} {
Simon> +			    _op .byte \
Simon> +				"$Dwarf::_constants(DW_CFA_advance_loc) + $delta" \
Simon> +				DW_CFA_advance_loc
Simon> +			}
Simon> +		    }
Simon> +
Simon> +		    DW_CFA_offset {
Simon> +			proc DW_CFA_offset {register offset} {
Simon> +			    _op .byte \
Simon> +				"$Dwarf::_constants(DW_CFA_offset) + $register" \
Simon> +				DW_CFA_offset
Simon> +			    _op .uleb128 $offset "offset"
Simon> +			}
Simon> +		    }
Simon> +
Simon> +		    DW_CFA_restore {
Simon> +			proc DW_CFA_restore {register} {
Simon> +			    _op .byte \
Simon> +				"$Dwarf::_constants(DW_CFA_restore) + $register" \
Simon> +				DW_CFA_restore
Simon> +			}
Simon> +		    }

These procs could just be defined at the namespace level and then these
constants just skipped here, like

    DW_CFA_advance_loc -
    DW_CFA_offset -
    DW_CFA_restore {
        # These need special handling and are predefined below.
    }

I think this would be a bit clearer.

Approved-By: Tom Tromey <tom@tromey.com>

thanks,
Tom