From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTP id B92D93854801 for ; Thu, 18 Mar 2021 21:15:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B92D93854801 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 1F582117B88; Thu, 18 Mar 2021 17:15:22 -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 rRWWs40kJvH2; Thu, 18 Mar 2021 17:15:22 -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 B6088117732; Thu, 18 Mar 2021 17:15:21 -0400 (EDT) From: Tom Tromey To: Tom Tromey Cc: Andrew Burgess , 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> <87r1kcxqx6.fsf@tromey.com> X-Attribution: Tom Date: Thu, 18 Mar 2021 15:15:21 -0600 In-Reply-To: <87r1kcxqx6.fsf@tromey.com> (Tom Tromey's message of "Thu, 18 Mar 2021 12:22:45 -0600") Message-ID: <87im5oxixi.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=-10.0 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 21:15:25 -0000 Andrew> Maybe I'm over thinking this, but you could change this to: Tom> Here's the definitive (IMNSHO) over-thinking approach. It works by Tom> having the helper proc set the necessary variables for the caller. One problem I thought of with that approach is that if you typo the code in one case, you could pick up a local variable set from some previous case. Switching this to use an array that's cleared after every opcode avoids this problem. Let me know what you think. Tom commit b437283334e0fca000a8260f26af95b1538df7d9 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 an array 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..5846a9dc862 100644 --- a/gdb/testsuite/lib/dwarf.exp +++ b/gdb/testsuite/lib/dwarf.exp @@ -890,6 +890,20 @@ namespace eval Dwarf { } } + # Assign elements from LINE to the elements of an array named + # "argvec" in the caller scope. The keys used are 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]" + } + + upvar argvec argvec + foreach var $args value [lreplace $line 0 0] { + set argvec($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. @@ -924,59 +938,72 @@ namespace eval Dwarf { set opcode [_map_name [lindex $line 0] _OP] _op .byte $_constants($opcode) $opcode + array unset argvec * switch -exact -- $opcode { DW_OP_addr { - _op .${addr_size}byte [lindex $line 1] + _get_args $line $opcode size + _op .${addr_size}byte $argvec(size) } DW_OP_regx { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode register + _op .uleb128 $argvec(register) } DW_OP_pick - DW_OP_const1u - DW_OP_const1s { - _op .byte [lindex $line 1] + _get_args $line $opcode const + _op .byte $argvec(const) } DW_OP_const2u - DW_OP_const2s { - _op .2byte [lindex $line 1] + _get_args $line $opcode const + _op .2byte $argvec(const) } DW_OP_const4u - DW_OP_const4s { - _op .4byte [lindex $line 1] + _get_args $line $opcode const + _op .4byte $argvec(const) } DW_OP_const8u - DW_OP_const8s { - _op .8byte [lindex $line 1] + _get_args $line $opcode const + _op .8byte $argvec(const) } DW_OP_constu { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode const + _op .uleb128 $argvec(const) } DW_OP_consts { - _op .sleb128 [lindex $line 1] + _get_args $line $opcode const + _op .sleb128 $argvec(const) } DW_OP_plus_uconst { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode const + _op .uleb128 $argvec(const) } DW_OP_piece { - _op .uleb128 [lindex $line 1] + _get_args $line $opcode size + _op .uleb128 $argvec(size) } DW_OP_bit_piece { - _op .uleb128 [lindex $line 1] - _op .uleb128 [lindex $line 2] + _get_args $line $opcode size offset + _op .uleb128 $argvec(size) + _op .uleb128 $argvec(offset) } DW_OP_skip - DW_OP_bra { - _op .2byte [lindex $line 1] + _get_args $line $opcode label + _op .2byte $argvec(label) } DW_OP_implicit_value { @@ -1000,45 +1027,37 @@ 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 + _op .${addr_size}byte $argvec(label) } else { - _op .${offset_size}byte $label + _op .${offset_size}byte $argvec(label) } - _op .sleb128 [lindex $line 2] + _op .sleb128 $argvec(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 + _op .${addr_size}byte $argvec(label) } else { - _op .${offset_size}byte $label + _op .${offset_size}byte $argvec(label) } } 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 $argvec(size) } DW_OP_bregx { - _op .uleb128 [lindex $line 1] - _op .sleb128 [lindex $line 2] + _get_args $line $opcode register offset + _op .uleb128 $argvec(register) + _op .sleb128 $argvec(offset) } default {