From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15988 invoked by alias); 20 May 2010 16:47:57 -0000 Received: (qmail 15976 invoked by uid 22791); 20 May 2010 16:47:55 -0000 X-SWARE-Spam-Status: No, hits=-5.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 20 May 2010 16:47:40 +0000 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4KGlcg6027057 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 20 May 2010 12:47:39 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o4KGlavC013705 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 20 May 2010 12:47:38 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o4KGlaWj027773; Thu, 20 May 2010 18:47:36 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o4KGlZ1g027772; Thu, 20 May 2010 18:47:35 +0200 Date: Thu, 20 May 2010 17:03:00 -0000 From: Jan Kratochvil To: Sergio Durigan Junior Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Forbid watchpoint on a constant value Message-ID: <20100520164735.GA25121@host0.dyn.jankratochvil.net> References: <201005181418.24324.sergiodj@redhat.com> <201005200210.27056.sergiodj@redhat.com> <20100520152941.GA19950@host0.dyn.jankratochvil.net> <201005201313.02076.sergiodj@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201005201313.02076.sergiodj@redhat.com> User-Agent: Mutt/1.5.20 (2009-08-17) X-IsSubscribed: yes 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 X-SW-Source: 2010-05/txt/msg00424.txt.bz2 On Thu, 20 May 2010 18:13:01 +0200, Sergio Durigan Junior wrote: > On Thursday 20 May 2010 12:29:42, Jan Kratochvil wrote: > > On Thu, 20 May 2010 07:10:26 +0200, Sergio Durigan Junior wrote: > > > + case BINOP_ASSIGN: > > > + case BINOP_ASSIGN_MODIFY: > > > + case OP_FUNCALL: > > > + case OP_OBJC_MSGCALL: > > > + case OP_F77_UNDETERMINED_ARGLIST: > > > + case UNOP_PREINCREMENT: > > > + case UNOP_POSTINCREMENT: > > > + case UNOP_PREDECREMENT: > > > + case UNOP_POSTDECREMENT: > > > > This is not a `const'/`pure' function, it has some side-effect of the > > assignment. I do not thing they should be caught as constant. > > Sorry, I didn't understand why they can't be considered constant in the > context of a watchpoint. echo 'int v;main(){}'|gcc -o 1 -x c - -g;./gdb -nx -ex 'p v' -ex 'watch v=1' -ex start -ex 'p v' ./1 The output of last -ex 'p v' will change if you include / not include -ex 'watch v=1' so that 'watch v=1' is not a NOP without meaning. Someone may use it in existing scripts to set variable `v' this way. I understand it is very broken idea to modify variables from watchpoint expression. Just I did not find it too useful to check here and when such patch could change the GDB behavior I find it more a disadvantage than an advantage. But I do not have a strong opinion on it. > > > + case BINOP_VAL: > > > + case BINOP_INCL: > > > + case BINOP_EXCL: > > > + case UNOP_PLUS: > > > + case UNOP_CAP: > > > + case UNOP_CHR: > > > + case UNOP_ORD: > > > + case UNOP_ABS: > > > + case UNOP_FLOAT: > > > + case UNOP_MAX: > > > + case UNOP_MIN: > > > + case UNOP_ODD: > > > + case UNOP_TRUNC: > > > > I do not see implemented evaluation of these, also their processing should > > have been probably moved to some m2-* file. > > Does it mean that I have to remove them from this list? There is a problem these operators have no implementation in the current GDB sources. Therefore one cannot verify what they do and if they are or they are not constant. How did you verify BINOP_VAL does not depend on some possibly changing value? The comments at their definition may not be always right. As they are also not useful to be used in an expression when no processing of them is implemented I find more dangerous to make some - possibly invalid - assumptions about them (that they are constant - they possibly may be implemented in a non-constant way in the future). The note about move to a different file was invalid, described in the very last comment of my mail. > > > + case UNOP_LOWER: > > > + case UNOP_UPPER: > > > + case UNOP_LENGTH: > > > + case UNOP_CARD: > > > + case UNOP_CHMAX: > > > + case UNOP_CHMIN: > > > > I do not see implemented evaluation of these, also their processing should > > have been probably moved to ... the already deleted Chill support files. > > Likewise. Likewise. > > > + case OP_INTERNALVAR: > > > > I would guess value of some of the internal variables can change. > > Is the user allowed to put a watchpoint on an internal variable? It seems to me so: (gdb) watch $a Watchpoint 2: $a Although the watchpoint does not get hit when $a gets modified during inferior run. Unaware why. > > > + case UNOP_HIGH: > > > > If it really should be here it could be moved into m2-* but this separation is > > already not strictly followed. > > Sorry, I'm afraid I didn't understand your comment. UNOP_HIGH can be probably included as I see now, sorry. There is ada-lang.c:ada_operator_check which contains language-specific (for ada in this case) operators. I thought processing of a language-specific operators should be in their specific language file. But this currently applies only to Ada operators from ada-operator.inc as they have OP_* numerical values possibly overlapping (*) other language-specific OP_* numerical values. Currently all the non-Ada operators are defined globally unique so it is safe enough to process them in a general file like you did in breakpoint.c. (*) As Ada is currently the only language with OP_* numerical values starting at OP_EXTENDED0 sure currently it does not overlap them. Thanks, Jan