Re: [PATCH][gdb/testsuite] Add .debug_loc support in dwarf assembler

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index 66f9844fe2e..a15eacfb46c 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -2369,6 +2369,102 @@ namespace eval Dwarf {
>  	aranges_end:
>      }
>  
> +    # Emit a .debug_loc entry.
> +
> +    proc loc_entry { start end location_description } {
> +	# Determine how to emit addresses.
> +	variable _addr_size
> +	if { $_addr_size == 8 } {
> +	    set addr_op .8byte
> +	} elseif { $_addr_size == 4 } {
> +	    set addr_op .4byte
> +	}
> +
> +	# Emit start and end address.
> +	_op $addr_op $start "Start address"
> +	_op $addr_op $end "End address"
> +
> +	declare_labels location_description_start
> +	declare_labels location_description_end
> +
> +	# Emit length of location description.
> +	set len "$location_description_end - $location_description_start"
> +	_op .2byte $len "Location description length"
> +
> +	# Tag start of location description.
> +	define_label $location_description_start
> +
> +	# Emit location description.
> +	variable _cu_version
> +	variable _cu_offset_size
> +	_location $location_description $_cu_version $_addr_size \
> +	    $_cu_offset_size
> +
> +	# Tag end of location description.
> +	define_label $location_description_end
> +    }
> +
> +    # Emit a DWARF .debug_loc contribution.
> +    #
> +    # OPTIONS is a list with an even number of elements containing
> +    # option-name and option-value pairs.
> +    # Current options are:
> +    # cu_is_64 0|1 - boolean indicating if references from location
> +    #                descriptions refer to a 64-bit DWARF CU.
> +    #                default = 0 (32-bit)
> +    # cu_version n - section version of DWARF CU referenced from location
> +    #                descriptions.
> +    #                default = 4
> +    #
> +    # BODY is Tcl code that emits the parts which make up the body of
> +    # the debug_loc contribution.  It is evaluated in the caller's context.
> +    # The following command is available for the BODY section:
> +    #
> +    #   loc_entry <start> <end> <location description>
> +    #     -- emit a .debug_loc entry
> +
> +    proc loc { options body } {

I would suggest using the same trick as used for rnglists: rename
loc_entry to _loc_entry (or something like that, to show it's
internal).  And use:

	with_override Dwarf::entry Dwarf::_loc_entry {
	    uplevel $body
	}

Users would then look like:

    loc { ... } {
	entry 0x123 0x456 {
	    ...
        }
    }

I think that looks good: because you are in the "loc" context, it is
obvious enough that entry is a location list entry.  I think that makes
it easy to write.

I would have suggested renaming "loc" to "loclist", because this is
actually generating a "location list", according to the standard.  But
that might become confusing with the DWARF5 "loclists"...

But other than that, the patch LGTM.

Simon