From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x529.google.com (mail-ed1-x529.google.com [IPv6:2a00:1450:4864:20::529]) by sourceware.org (Postfix) with ESMTPS id E7F4538618E2 for ; Thu, 18 Mar 2021 17:15:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E7F4538618E2 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-ed1-x529.google.com with SMTP id j3so7590411edp.11 for ; Thu, 18 Mar 2021 10:15:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=zeC30ejPxF/PIoOcG0eky8wyrmZN2yaAy4y1/HUhxjw=; b=RIL5lAdnbgosMvOalm6MrykNH7WU+mG1Pq7J9PgK8a5hE8ZMCrFBLhl3OdbuEXWHpR KP/+zisVkrOZ+bp5S0bNTxBBB8HGbnjXWBNvd2zCj1ALlULY3rwgO3hbYAH1doozJyrU u8qDVCPbaqzAKp3YS6dd3K+UlGQ7ucHlId5appvoaL4E/OSYWVYid2cUseU++KAEfYJk CupxYK1s94jXxB+aib4OHnr2y3mkDs/as/WClvnwmt8MsS+lq0UfM+Hmr3TwZNOtnDM9 n0wN3PPIolEsCRKhaci/2q4XC7xwkeE5tpAuz/ydarSifMkGDejwuuHhC5ltGveDLTt5 lBTA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=zeC30ejPxF/PIoOcG0eky8wyrmZN2yaAy4y1/HUhxjw=; b=ZLlGDsUu1jbDMsgz4EaIondgU9d1RlNNg4YeJmaNjOFkK4W/pv3BOphZVZmI42VBGo brJldHZ7mhv8Nr2UBZThaK87iNN78qzmoV8lppd8QoAmPNpvoLv3FO9bmVUC4i+OQy9U RUVT4ijg1ZWB2axy5hin22ooPC9jYY5+MDZOlauMt6i34uN/kTJW8z2oBi4KQuKe37gq Zp1fv8BCgLBIW1EZRVFFk7DR9RA+fi7x6AdHxNhfo2aLnJmScKjNiKidCVTaVwluHv0P B/uyRcLI4kH+3nNRHpQuzqXSV2x/9qDu3SyzV3+Fnf4I1dpMN/nOPQwVlwoLkhUqJDIc y2kg== X-Gm-Message-State: AOAM533ITsbzviz2v15Foxv1AYcFljBHEGmWDQeGUn0IGm6USq9FbDmN JVvCjLBjyX+muKzBPxn8UMO8Xz8Hr6FKpw== X-Google-Smtp-Source: ABdhPJyHe0e2flnq1rz1WAI2505PNiZtsuATgsmbYJ9wLt7FGzxspKoLKEMufD7XPzNF1L7LVWqpBg== X-Received: by 2002:a05:6402:22f6:: with SMTP id dn22mr4830871edb.214.1616087756973; Thu, 18 Mar 2021 10:15:56 -0700 (PDT) Received: from localhost (host165-120-118-227.range165-120.btcentralplus.com. [165.120.118.227]) by smtp.gmail.com with ESMTPSA id q10sm2553720eds.67.2021.03.18.10.15.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Mar 2021 10:15:56 -0700 (PDT) Date: Thu, 18 Mar 2021 17:15:55 +0000 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Add some error checking to DWARF assembler Message-ID: <20210318171555.GF5520@embecosm.com> References: <20210318155826.3703918-1-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210318155826.3703918-1-tromey@adacore.com> X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 17:13:08 up 7 days, 24 min, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, 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 17:15:59 -0000 * Tom Tromey [2021-03-18 09:58:26 -0600]: > 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. I removed some existing checks in the name of > normalization; while the new messages are a bit worse (being > one-size-fits-all), I think this doesn't matter hugely, since this > should only be seen for test suite bugs. > > gdb/testsuite/ChangeLog > 2021-03-18 Tom Tromey > > * lib/dwarf.exp (Dwarf::_check_n): New proc. > (Dwarf::_location): Use it. > --- > gdb/testsuite/ChangeLog | 5 +++++ > gdb/testsuite/lib/dwarf.exp | 33 +++++++++++++++++++++++---------- > 2 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp > index f8fbd381810..64b9e263f10 100644 > --- a/gdb/testsuite/lib/dwarf.exp > +++ b/gdb/testsuite/lib/dwarf.exp > @@ -890,6 +890,13 @@ namespace eval Dwarf { > } > } > > + # Require N arguments for the opcode. > + proc _check_n {line op n} { > + if {[llength $line] != $n + 1} { > + error "$op requires $n arguments" > + } > + } > + > # 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,56 +933,68 @@ namespace eval Dwarf { > > switch -exact -- $opcode { > DW_OP_addr { > + _check_n $line $opcode 1 > _op .${addr_size}byte [lindex $line 1] > } > > DW_OP_regx { > + _check_n $line $opcode 1 > _op .uleb128 [lindex $line 1] > } > > DW_OP_pick - > DW_OP_const1u - > DW_OP_const1s { > + _check_n $line $opcode 1 > _op .byte [lindex $line 1] > } > > DW_OP_const2u - > DW_OP_const2s { > + _check_n $line $opcode 1 > _op .2byte [lindex $line 1] > } > > DW_OP_const4u - > DW_OP_const4s { > + _check_n $line $opcode 1 > _op .4byte [lindex $line 1] > } > > DW_OP_const8u - > DW_OP_const8s { > + _check_n $line $opcode 1 > _op .8byte [lindex $line 1] > } > > DW_OP_constu { > + _check_n $line $opcode 1 > _op .uleb128 [lindex $line 1] > } > DW_OP_consts { > + _check_n $line $opcode 1 > _op .sleb128 [lindex $line 1] > } > > DW_OP_plus_uconst { > + _check_n $line $opcode 1 > _op .uleb128 [lindex $line 1] > } > > DW_OP_piece { > + _check_n $line $opcode 1 > _op .uleb128 [lindex $line 1] > } > > DW_OP_bit_piece { > + _check_n $line $opcode 2 > _op .uleb128 [lindex $line 1] > _op .uleb128 [lindex $line 2] > } > > DW_OP_skip - > DW_OP_bra { > + _check_n $line $opcode 1 > _op .2byte [lindex $line 1] > } > > @@ -1000,9 +1019,7 @@ namespace eval Dwarf { > > DW_OP_implicit_pointer - > DW_OP_GNU_implicit_pointer { > - if {[llength $line] != 3} { > - error "usage: $opcode LABEL OFFSET" > - } > + _check_n $line $opcode 2 > > # Here label is a section offset. > set label [lindex $line 1] > @@ -1015,9 +1032,7 @@ namespace eval Dwarf { > } > > DW_OP_GNU_variable_value { > - if {[llength $line] != 2} { > - error "usage: $opcode LABEL" > - } > + _check_n $line $opcode 1 > > # Here label is a section offset. > set label [lindex $line 1] > @@ -1029,14 +1044,12 @@ namespace eval Dwarf { > } > > DW_OP_deref_size { > - if {[llength $line] != 2} { > - error "usage: DW_OP_deref_size SIZE" > - } > - > + _check_n $line $opcode 1 Maybe I'm over thinking this, but you could change this to: _check_n $line $opcode { SIZE } And for DW_OP_implicit_pointer above you would do: _check_n $line $opcode { LABEL OFFSET } Then in _check_n you can use the length of the final list to validate the argument count, and if its wrong you can still print the helpful text.... Just a thought. Either way, more error checking is good. Thanks, Andrew