From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80337 invoked by alias); 4 Jan 2017 20:54:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 80328 invoked by uid 89); 4 Jan 2017 20:54:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=Hx-languages-length:5180, competing, H*f:sk:ef39b7b, disagreeing X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Jan 2017 20:53:58 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cOsZI-0006OX-V2 from Luis_Gustavo@mentor.com ; Wed, 04 Jan 2017 12:53:56 -0800 Received: from [172.30.9.137] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 4 Jan 2017 12:53:53 -0800 Reply-To: Luis Machado Subject: Re: [PATCH] D: Fix crash when expression debugging References: <5806a075-dc30-b664-a834-a633b3b27256@codesourcery.com> To: Iain Buclaw CC: GDB Patches From: Luis Machado Message-ID: Date: Wed, 04 Jan 2017 20:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-01.mgc.mentorg.com (147.34.90.201) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00064.txt.bz2 On 01/04/2017 02:38 PM, Iain Buclaw wrote: > On 4 January 2017 at 20:48, Luis Machado wrote: >> On 01/04/2017 01:38 PM, Iain Buclaw wrote: >>> >>> On 4 January 2017 at 17:08, Luis Machado >>> wrote: >>>> >>>> On 01/03/2017 06:02 PM, Iain Buclaw wrote: >>>>> >>>>> >>>>> This patch fixes a crash found on "p *(type *)0x1234" when using "set >>>>> debug expression 1". >>>>> >>>>> While casting works as expected with expression debugging turned off, >>>>> this seems to be an indication that I'm infact doing something wrong >>>>> in the building of the expression. >>>>> >>>>> (gdb) print *(int*)(0x0) >>>>> Dump of expression @ 0x12db320, before conversion to prefix form: >>>>> Language d, 11 elements, 16 bytes each. >>>>> Index Opcode Hex Value String Value >>>>> 0 OP_TYPE 87 W............... >>>>> 1 20114800 p.2............. >>>>> 2 OP_TYPE 87 W............... >>>>> 3 OP_LONG 38 &............... >>>>> 4 19696640 ..,............. >>>>> 5 OP_NULL 0 ................ >>>>> 6 OP_LONG 38 &............... >>>>> 7 UNOP_CAST 51 3............... >>>>> 8 20114800 p.2............. >>>>> 9 UNOP_CAST 51 3............... >>>>> 10 UNOP_IND 61 =............... >>>>> Dump of expression @ 0x12db320, after conversion to prefix form: >>>>> Expression: `*(int *) 0' >>>>> Language d, 11 elements, 16 bytes each. >>>>> >>>>> >>>>> 0 UNOP_IND >>>>> 1 UNOP_CAST Type @0x132ed70 (int *) >>>>> 4 OP_LONG Type @0x12c8c00 (int), value 0 >>>>> (0x0) >>>>> 8 Unknown format >>>>> 9 UNOP_CAST Type @0x3d (Segmentation fault >>>>> >>>>> >>>>> Looks like using UNOP_CAST_TYPE is the right thing to do here, as the >>>>> TypeExp handler has wrapped the type around a pair of OP_TYPE opcodes. >>>>> >>>>> >>>>> dlang-debug-expr.patch >>>>> >>>>> >>>>> 2017-01-04 Iain Buclaw >>>>> >>>>> gdb/ChangeLog: >>>>> >>>>> * d-exp.y (CastExpression): Emit UNOP_CAST_TYPE. >>>>> >>>>> gdb/testsuite/ChangeLog: >>>>> >>>>> * gdb.dlang/debug-expr.exp: New file. >>>>> >>>>> --- >>>>> diff --git a/gdb/d-exp.y b/gdb/d-exp.y >>>>> index 077e645..91d15f2 100644 >>>>> --- a/gdb/d-exp.y >>>>> +++ b/gdb/d-exp.y >>>>> @@ -321,15 +321,12 @@ UnaryExpression: >>>>> >>>>> CastExpression: >>>>> CAST_KEYWORD '(' TypeExp ')' UnaryExpression >>>>> - { write_exp_elt_opcode (pstate, UNOP_CAST); >>>>> - write_exp_elt_type (pstate, $3); >>>>> - write_exp_elt_opcode (pstate, UNOP_CAST); } >>>>> + { write_exp_elt_opcode (pstate, UNOP_CAST_TYPE); } >>>>> /* C style cast is illegal D, but is still recognised in >>>>> the grammar, so we keep this around for convenience. */ >>>>> | '(' TypeExp ')' UnaryExpression >>>>> - { write_exp_elt_opcode (pstate, UNOP_CAST); >>>>> - write_exp_elt_type (pstate, $2); >>>>> - write_exp_elt_opcode (pstate, UNOP_CAST); } >>>>> + { write_exp_elt_opcode (pstate, UNOP_CAST_TYPE); } >>>>> + >>>>> ; >>>>> >>>>> PowExpression: >>>>> diff --git a/gdb/testsuite/gdb.dlang/debug-expr.exp >>>>> b/gdb/testsuite/gdb.dlang/debug-expr.exp >>>>> new file mode 100644 >>>>> index 0000000..3bb2c09 >>>>> --- /dev/null >>>>> +++ b/gdb/testsuite/gdb.dlang/debug-expr.exp >>>>> @@ -0,0 +1,40 @@ >>>>> +# Copyright 2017 Free Software Foundation, Inc. >>>>> + >>>>> +# This program is free software; you can redistribute it and/or modify >>>>> +# it under the terms of the GNU General Public License as published by >>>>> +# the Free Software Foundation; either version 3 of the License, or >>>>> +# (at your option) any later version. >>>>> +# >>>>> +# This program is distributed in the hope that it will be useful, >>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> +# GNU General Public License for more details. >>>>> +# >>>>> +# You should have received a copy of the GNU General Public License >>>>> +# along with this program. If not, see . >>>>> + >>>>> +# Test "set debug expr 1" on d expressions. >>>>> + >>>>> +if { [skip_d_tests] } { continue } >>>> >>>> >>>> >>>> We should output a message: >>>> >>>> untested "skipping d language tests" >>>> >>>> It may be more reasonable to just return instead of continuing? The >>>> effect >>>> will probably be the same, but it is a bit confusing to read "continue" >>>> without a visible loop. >>>> >>> >>> I did a quick grep, and it seems like everyone is skippingtests in >>> this way except for gdb.ada and gdb.btrace which are doing { return -1 >>> } >>> >>> >> >> That's a bit of a stretch. Take, for example, a few of the examples in >> gdb.base. You will see a number of them returning. >> >> The problem here is inheriting past confusing practices when we use some >> existing files to create new ones, which is not your fault really. I'm >> guilty myself. :-) >> > > Yes indeed. I wasn't disagreeing, just questioning the two competing > ways of returning. > > I will update to use return and push this in then if there's no > disagreement. :-) > Hopefully someone will chime in for a second opinion. :-)