From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [205.232.38.15]) by sourceware.org (Postfix) with ESMTP id 005A93848011 for ; Thu, 18 Mar 2021 18:22:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 005A93848011 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id CD172117D41; Thu, 18 Mar 2021 14:22:46 -0400 (EDT) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id tV06cqL7hjq7; Thu, 18 Mar 2021 14:22:46 -0400 (EDT) Received: from murgatroyd (71-211-137-228.hlrn.qwest.net [71.211.137.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 6E922117D30; Thu, 18 Mar 2021 14:22:46 -0400 (EDT) From: Tom Tromey To: Andrew Burgess Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH] Add some error checking to DWARF assembler References: <20210318155826.3703918-1-tromey@adacore.com> <20210318171555.GF5520@embecosm.com> X-Attribution: Tom Date: Thu, 18 Mar 2021 12:22:45 -0600 In-Reply-To: <20210318171555.GF5520@embecosm.com> (Andrew Burgess's message of "Thu, 18 Mar 2021 17:15:55 +0000") Message-ID: <87r1kcxqx6.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Mar 2021 18:22:48 -0000 Andrew> Maybe I'm over thinking this, but you could change this to: Here's the definitive (IMNSHO) over-thinking approach. It works by having the helper proc set the necessary variables for the caller. Tom commit f223a278829665589f47cd39724148fa0a4ca5ce Author: Tom Tromey Date: Thu Mar 18 09:54:27 2021 -0600 Add some error checking to DWARF assembler I had written a DWARF location expression like DW_OP_const1u DW_OP_stack_value ... and was surprised to see that the DW_OP_stack_value didn't appear in the "readelf" output. The problem here is that DW_OP_const1u requires an operand, but neither the DWARF assembler nor gas diagnosed this problem. This patch adds some checking to Dwarf::_location to try to avoid this in the future. The checking is done via a helper proc that also dissects the argument list and sets local variables in the caller's frame. gdb/testsuite/ChangeLog 2021-03-18 Tom Tromey * lib/dwarf.exp (Dwarf::_get_args): New proc. (Dwarf::_location): Use it. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index c1d7fec6cc4..f76bb167502 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2021-03-18 Tom Tromey + + * lib/dwarf.exp (Dwarf::_get_args): New proc. + (Dwarf::_location): Use it. + 2021-03-17 Simon Marchi Pedro Alves diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp index f8fbd381810..1c672351dd7 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -890,6 +890,19 @@ namespace eval Dwarf { } } + # Assign elements from LINE to the variables named in ARGS. + # If the wrong number of elements appear in LINE, error. + proc _get_args {line op args} { + if {[llength $line] != [llength $args] + 1} { + error "usage: $op [string toupper $args]" + } + + foreach var $args value [lreplace $line 0 0] { + upvar $var __$var + set __$var $value + } + } + # This is a miniature assembler for location expressions. It is # suitable for use in the attributes to a DIE. Its output is # prefixed with "=" to make it automatically use DW_FORM_block. @@ -926,57 +939,69 @@ namespace eval Dwarf { switch -exact -- $opcode { DW_OP_addr { - _op .${addr_size}byte [lindex $line 1] + _get_args $line $opcode size + _op .${addr_size}byte $size } DW_OP_regx { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode register + _op .uleb128 $register } DW_OP_pick - DW_OP_const1u - DW_OP_const1s { - _op .byte [lindex $line 1] + _get_args $line $opcode const + _op .byte $const } DW_OP_const2u - DW_OP_const2s { - _op .2byte [lindex $line 1] + _get_args $line $opcode const + _op .2byte $const } DW_OP_const4u - DW_OP_const4s { - _op .4byte [lindex $line 1] + _get_args $line $opcode const + _op .4byte $const } DW_OP_const8u - DW_OP_const8s { - _op .8byte [lindex $line 1] + _get_args $line $opcode const + _op .8byte $const } DW_OP_constu { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode const + _op .uleb128 $const } DW_OP_consts { - _op .sleb128 [lindex $line 1] + _get_args $line $opcode const + _op .sleb128 $const } DW_OP_plus_uconst { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode const + _op .uleb128 $const } DW_OP_piece { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode size + _op .uleb128 $size } DW_OP_bit_piece { - _op .uleb128 [lindex $line 1] - _op .uleb128 [lindex $line 2] + _get_args $line $opcode size offset + _op .uleb128 $size + _op .uleb128 $offset } DW_OP_skip - DW_OP_bra { - _op .2byte [lindex $line 1] + _get_args $line $opcode label + _op .2byte $label } DW_OP_implicit_value { @@ -1000,27 +1025,21 @@ namespace eval Dwarf { DW_OP_implicit_pointer - DW_OP_GNU_implicit_pointer { - if {[llength $line] != 3} { - error "usage: $opcode LABEL OFFSET" - } + _get_args $line $opcode label offset # Here label is a section offset. - set label [lindex $line 1] if { $dwarf_version == 2 } { _op .${addr_size}byte $label } else { _op .${offset_size}byte $label } - _op .sleb128 [lindex $line 2] + _op .sleb128 $offset } DW_OP_GNU_variable_value { - if {[llength $line] != 2} { - error "usage: $opcode LABEL" - } + _get_args $line $opcode label # Here label is a section offset. - set label [lindex $line 1] if { $dwarf_version == 2 } { _op .${addr_size}byte $label } else { @@ -1029,16 +1048,14 @@ namespace eval Dwarf { } DW_OP_deref_size { - if {[llength $line] != 2} { - error "usage: DW_OP_deref_size SIZE" - } - - _op .byte [lindex $line 1] + _get_args $line $opcode size + _op .byte $size } DW_OP_bregx { - _op .uleb128 [lindex $line 1] - _op .sleb128 [lindex $line 2] + _get_args $line $opcode register offset + _op .uleb128 $register + _op .sleb128 $offset } default {