From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24867 invoked by alias); 12 May 2009 14:37:21 -0000 Received: (qmail 24850 invoked by uid 22791); 12 May 2009 14:37:19 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.158) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 May 2009 14:37:14 +0000 Received: from baal.u-strasbg.fr (baal.u-strasbg.fr [IPv6:2001:660:2402::41]) by mailhost.u-strasbg.fr (8.14.2/jtpda-5.5pre1) with ESMTP id n4CEauaN010577 ; Tue, 12 May 2009 16:36:56 +0200 (CEST) Received: from mailserver.u-strasbg.fr (ms1.u-strasbg.fr [IPv6:2001:660:2402:d::10]) by baal.u-strasbg.fr (8.14.0/jtpda-5.5pre1) with ESMTP id n4CEatiJ057875 ; Tue, 12 May 2009 16:36:55 +0200 (CEST) (envelope-from muller@ics.u-strasbg.fr) Received: from d620muller (www-ics.u-strasbg.fr [130.79.210.225]) (user=mullerp mech=LOGIN) by mailserver.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id n4CEasg0071426 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NO) ; Tue, 12 May 2009 16:36:55 +0200 (CEST) (envelope-from muller@ics.u-strasbg.fr) From: "Pierre Muller" To: "'Ulrich Weigand'" Cc: , "'Pedro Alves'" , "'Eli Zaretskii'" References: <005001c9d17f$302199d0$9064cd70$@u-strasbg.fr> from "Pierre Muller" at May 10, 2009 04:54:13 PM <200905121300.n4CD0vMQ003287@d12av02.megacenter.de.ibm.com> In-Reply-To: <200905121300.n4CD0vMQ003287@d12av02.megacenter.de.ibm.com> Subject: RE: [RFC] Remove i386 low level debug register function from nm- header file. Date: Tue, 12 May 2009 14:37:00 -0000 Message-ID: <003701c9d30f$1cea0a00$56be1e00$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable 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: 2009-05/txt/msg00245.txt.bz2 Thanks for the feedback. > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Ulrich Weigand > Envoy=E9=A0: Tuesday, May 12, 2009 3:01 PM > =C0=A0: Pierre Muller > Cc=A0: gdb-patches@sourceware.org; 'Pedro Alves'; 'Eli Zaretskii' > Objet=A0: Re: [RFC] Remove i386 low level debug register function from > nm- header file. >=20 > Pierre Muller wrote: >=20 > > Following my first RFC > > http://sourceware.org/ml/gdb-patches/2009-03/msg00197.html > > > > and the following answers, I resubmit > > a new version where all i386 low level debug register > > is contained in a single struct defined in the > > new i386-nat.h header (to comply with DOS file name restrictions > > as asked for by Eli). >=20 > This looks good to me. However, now that you've moved the > prototypes to i386-nat.h, there is no longer any need for > the config/i386/nm-i386.h file -- please get rid of that file > (by removing the places where it is included into other nm- > files). OK, but how do I commit a file deletion? I never did this before! > Also, it seems to me that now there is no need any more for > the I386_USE_GENERIC_WATCHPOINTS define -- targets that use > watchpoints call the i386_use_watchpoints routines, those > that don't, do not. Seems reasonable indeed! =20 > I think you should provide everything in i386-nat.c (and > the new i386-nat.h) unconditionally, and eliminate the > various definitions of I386_USE_GENERIC_WATCHPOINTS in the > nm- header files. The only drawback is that "maint show-debug--regs" command will then also appear on target that do not support debug registers... Anyhow, it will just be a no-op in that case. Is that a problem? > Note that in combination with removing nm-i386.h, this > actually makes several nm- files completely empty, so they > should be removed as well. >=20 > > 2009-05-10 Pierre Muller > > > > Remove all i386 debug register low level macros in config tm > files. >=20 > Should read "nm" files, not tm. Yes of course, thanks for catching this! > > +/* Targets should define this to use the generic x86 watchpoint > support. > > */ > > +#ifdef I386_USE_GENERIC_WATCHPOINTS >=20 > As mentioned above, please get rid of this conditional ... >=20 > > + > > +/* Add watchpoint methods to the provided target_ops. > > + Targets defining I386_USE_GENERIC_WATCHPOINTS should call > > + this. */ >=20 > ... as well as the second sentence of the comment. >=20 > > /* Support for 8-byte wide hw watchpoints. */ > > #ifndef TARGET_HAS_DR_LEN_8 > > -#define TARGET_HAS_DR_LEN_8 0 > > +#define TARGET_HAS_DR_LEN_8 (i386_dr_low.debug_register_length =3D=3D = 8) > > #endif >=20 > Nobody should be defining this any more, so please eliminate > the #ifndef ... conditional. OK, will do. =20 > > Index: src/gdb/i386fbsd-nat.c > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > RCS file: /cvs/src/src/gdb/i386fbsd-nat.c,v > > retrieving revision 1.18 > > @@ -126,7 +127,19 @@ _initialize_i386fbsd_nat (void) > > > > /* Add some extra features to the common *BSD/i386 target. */ > > t =3D i386bsd_target (); > > + > > +#ifdef I386_USE_GENERIC_WATCHPOINTS > > + > > i386_use_watchpoints (t); > > + > > + i386_dr_low.set_control =3D i386bsd_dr_set_control; > > + i386_dr_low.set_addr =3D i386bsd_dr_set_addr; > > + i386_dr_low.reset_addr =3D i386bsd_dr_reset_addr; > > + i386_dr_low.get_status =3D i386bsd_dr_get_status; > > + i386_set_debug_register_length (4); > > +#endif /* I386_USE_GENERIC_WATCHPOINTS */ >=20 >=20 > In this case, because the header file currently conditionally defines > I386_USE_GENERIC_WATCHPOINTS: >=20 > #ifdef HAVE_PT_GETDBREGS > #define I386_USE_GENERIC_WATCHPOINTS > #endif >=20 > you should move that condition to the i386fbsd-nat.c file: >=20 > #ifdef HAVE_PT_GETDBREGS > i386_use_watchpoints (t); >=20 > ... > #endif So freebsd with old kernels (not supporting PT_GETDBREGS),=20 will typically have the non-operational "maint show-debug-reg" command. =20 > > diff -u -p -r1.189 windows-nat.c > > --- windows-nat.c 17 Apr 2009 15:44:28 -0000 1.189 > > +++ windows-nat.c 8 May 2009 14:02:25 -0000 > > @@ -2166,8 +2171,22 @@ init_windows_ops (void) > > windows_ops.to_has_execution =3D 1; > > windows_ops.to_pid_to_exec_file =3D windows_pid_to_exec_file; > > windows_ops.to_get_ada_task_ptid =3D windows_get_ada_task_ptid; > > + > > +#ifdef I386_USE_GENERIC_WATCHPOINTS > > + > > i386_use_watchpoints (&windows_ops); > > > > + i386_dr_low.set_control =3D cygwin_set_dr7; > > + i386_dr_low.set_addr =3D cygwin_set_dr; > > + i386_dr_low.reset_addr =3D NULL; > > + i386_dr_low.get_status =3D cygwin_get_dr6; >=20 > Here, it seems every user of windows-nat.c unconditionally > defines I386_USE_GENERIC_WATCHPOINTS, so there's no point in > adding the #ifdef's to this file ... I don't know here, the problem is that windows-nat is full of i386 specific code that should be moved to i386-windows-nat and/or amd64-windows-nat so that supporting other processors will be easier. Thanks Ulrich, I will try to submit a new version promptly. Pierre