[RFA] patch to add 'maint profile-gdb' command

[RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

This is a refresh of Tom Tromey's gdb profiling patch, originally here:
	http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00022.html

Instead of profiling all of gdb, Tom's patch lets you profile a
specific command (or commands).  You enable profiling with the
'maint profile-gdb on' before the command(s) of interest, and
'maint profile-gdb off' (or exit) when you're finished.

configure and config.in both need to be regenerated after applying
this patch.  Your build must be compiled with --enable-profiling
for this feature to be enabled.

My only comments on this patch are (1) the documentation entry
could note that your gmon.out file will be overwritten each time
gdb is started, even if you don't do a profile-gdb on command[1], and
(2) the configure.in check for $enable_profiling could be embedded
in the AC_ARG_ENABLE() autoconf call.  It doesn't make any practical
difference, but it looks like tradition in gdb's configure.in is
to include this code inside the AC_ARG_ENABLE call.

 [1]  A bit of profiling happens before it can be turned
      off in captured_main().  This initial profiling will overwrite
      away any existing gmon.out.  At least it does with the gprof
      on Linux and FreeBSD systems.

No testsuite regressions are added with this patch.

This patch does not require approval for the 5.1 branch - it is
not something end users have cause to enable.

This patch does add a couple of ifdefs in main.c, aint.c to guard
the code, but this is necessary.  Obviously you can't compile in
profiling all the time (performance, portability), and you can't
make calls to the profiling system calls if you aren't compiling
-pg.

Jason
From jason-swarelist@molenda.com Mon Sep 10 00:36:00 2001
From: Jason Molenda <jason-swarelist@molenda.com>
To: gdb-patches@sources.redhat.com
Subject: Typeo in NEWS
Date: Mon, 10 Sep 2001 00:36:00 -0000
Message-id: <20010910003611.A22499@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00126.html
Content-length: 249

The NEWS file claims that gdb's code has been converted to "ANS/ISO C".
It should either be "ANSI/ISO C", or more correctly, just "ISO C".  

I won't argue strenuously for "ISO" over "ANSI/ISO", but the patch
I'm including implements the latter.

J
From eliz@is.elta.co.il Mon Sep 10 00:50:00 2001
From: Eli Zaretskii <eliz@is.elta.co.il>
To: Jason Molenda <jason-swarelist@molenda.com>
Cc: gdb-patches@sources.redhat.com, Tom Tromey <tromey@cygnus.com>
Subject: Re: [RFA] patch to add 'maint profile-gdb' command
Date: Mon, 10 Sep 2001 00:50:00 -0000
Message-id: <Pine.SUN.3.91.1010910094003.18194H-100000@is>
References: <20010910003022.A21681@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00127.html
Content-length: 1698

On Mon, 10 Sep 2001, Jason Molenda wrote:

> This is a refresh of Tom Tromey's gdb profiling patch, originally here:
> 	http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00022.html

Thanks!  I think it's a very useful feature, but I have a few minor 
comments.

> My only comments on this patch are (1) the documentation entry
> could note that your gmon.out file will be overwritten each time
> gdb is started, even if you don't do a profile-gdb on command[1], and
> (2) the configure.in check for $enable_profiling could be embedded
> in the AC_ARG_ENABLE() autoconf call.  It doesn't make any practical
> difference, but it looks like tradition in gdb's configure.in is
> to include this code inside the AC_ARG_ENABLE call.
> 
>  [1]  A bit of profiling happens before it can be turned
>       off in captured_main().  This initial profiling will overwrite
>       away any existing gmon.out.  At least it does with the gprof
>       on Linux and FreeBSD systems.

Isn't it better to use monstartup instead of moncontrol and -pg?  It 
looks like using monstartup could solve several problems:

  - you don't overwrite gmon.out unless you actually profile
  - you can link without -pg
  - you can make this option available by default, since it is invisible 
    unless you actually profile (I dislike features that require a 
    rebuild to become available)

We will have to add Autoconf tests to see if monstartup is available, but 
I think using moncontrol if --enable-profiling is given is not safe 
enough anyway.

In addition, I think the commands this patch adds should be documented in 
gdb.texinfo as well, since all commands one can type at GDB's prompt 
should be described there.

Re: [RFA] patch to add 'maint profile-gdb' command

[Andrew Cagney]

> This is a refresh of Tom Tromey's gdb profiling patch, originally here:
> 	http://sources.redhat.com/ml/gdb-patches/2000-q1/msg00022.html
> 
> Instead of profiling all of gdb, Tom's patch lets you profile a
> specific command (or commands).  You enable profiling with the
> 'maint profile-gdb on' before the command(s) of interest, and
> 'maint profile-gdb off' (or exit) when you're finished.
> 
> configure and config.in both need to be regenerated after applying
> this patch.  Your build must be compiled with --enable-profiling
> for this feature to be enabled.

some brief, one handed, notes :-)

i think the command should always be present.  looking at the patch, it 
appears to have started out that way.

a check of *BSD's man pages also points to monstartup() and profil().  i 
dont think you should be trying to implement those.  however, i do think 
think it is important to ensure that the cli interface doesnt preclude 
adding them to the command set.  hence, i wonder if ``-gdb'' is needed 
in the command name?  Perhaps ``(gdb) maint profile control [on|off]''? 
  This is really a cli maintainer question and outside my domain.

are you sure no changes to maint.exp are needed? not needing them feels 
wrong.

suggest adding a ``post 5.1'' line to news - otherwise i'll get 
confused.  thanks for thinking of the news file.

(Eli?) should the doco include a reference to moncontrol(3)?

suggest a comment against the moncontrol() call in main.c explaining the 
problems.  would having the call in main() be better - turn it off at 
the earliest possible moment?

gdb is pure iso c, the params forward decl isn't needed.  you may want 
to look at http://sources.redhat.com/gdb/ari/ for a check list of things 
recently zapped from gdb.

and thanks for picking this up, get the above sorted out and the patch 
proper becomes obvious.

enjoy,
	andrew

> My only comments on this patch are (1) the documentation entry
> could note that your gmon.out file will be overwritten each time
> gdb is started, even if you don't do a profile-gdb on command[1], and
> (2) the configure.in check for $enable_profiling could be embedded
> in the AC_ARG_ENABLE() autoconf call.  It doesn't make any practical
> difference, but it looks like tradition in gdb's configure.in is
> to include this code inside the AC_ARG_ENABLE call.
> 
>  [1]  A bit of profiling happens before it can be turned
>       off in captured_main().  This initial profiling will overwrite
>       away any existing gmon.out.  At least it does with the gprof
>       on Linux and FreeBSD systems.
> 
> No testsuite regressions are added with this patch.
> 
> This patch does not require approval for the 5.1 branch - it is
> not something end users have cause to enable.
> 
> This patch does add a couple of ifdefs in main.c, aint.c to guard
> the code, but this is necessary.  Obviously you can't compile in
> profiling all the time (performance, portability), and you can't
> make calls to the profiling system calls if you aren't compiling
> -pg.
> 
> Jason
> 
> 

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

On Mon, 10 Sep 2001, Andrew Cagney wrote:

> a check of *BSD's man pages also points to monstartup() and profil().  i 
> dont think you should be trying to implement those.

I'm not sure I understand--are you saying monstartup shouldn't be used by 
this feature?  If so, why not?  I think it should be used if available, 
since it avoids the annoying problem of profiling some random startup 
code.

> (Eli?) should the doco include a reference to moncontrol(3)?

It's not clear to me why this is needed.  Is this issue important for 
some reason?

>  would having the call in main() be better - turn it off at 
> the earliest possible moment?

How about using __attribute__((constructor)) if available?

Re: [RFA] patch to add 'maint profile-gdb' command

[Andrew Cagney]

> On Mon, 10 Sep 2001, Andrew Cagney wrote:
> 
> 
>> a check of *BSD's man pages also points to monstartup() and profil().  i 
>> dont think you should be trying to implement those.
> 
> 
> I'm not sure I understand--are you saying monstartup shouldn't be used by 
> this feature?  If so, why not?  I think it should be used if available, 
> since it avoids the annoying problem of profiling some random startup 
> code.

no, just that i dont think i should be asking jason to add the mechanism.

andrew

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Mon, Sep 10, 2001 at 11:48:22AM -0400, Andrew Cagney wrote:

> some brief, one handed, notes :-)

Oh jeez, don't tell me you broke your arm again?!  :-)

> i think the command should always be present.  looking at the patch, it 
> appears to have started out that way.

I'd disagree.  First, these functions are not very portable.  Our
very own RH 6.2 box that is sourceware.cygnus.com does not have
moncontrol() or monstart(), and -pg doesn't seem to be usable at
all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
fine.  Second, you definitely don't want to compile gdb with -pg
by default - the compiler inserts a bunch of bookkeeping code and
functions, and that'll be a real performance penalty.

> hence, i wonder if ``-gdb'' is needed 
> in the command name?  Perhaps ``(gdb) maint profile control [on|off]''? 
>   This is really a cli maintainer question and outside my domain.

Personally, I think it could just be "maint profile [on|off]", but that
could be ambiguous - are you profiling gdb or its inferior?

> are you sure no changes to maint.exp are needed? not needing them feels 
> wrong.

The "help maint" test in maint.exp globs it up.  You don't want to include
a check for maint profile-gdb in here because you'd get a FAIL when gdb
was not configured --enable-profiling.

> suggest adding a ``post 5.1'' line to news - otherwise i'll get 
> confused.  thanks for thinking of the news file.

All credit goes to Tom, of course.

> (Eli?) should the doco include a reference to moncontrol(3)?

I don't see what you're thinking of.

> suggest a comment against the moncontrol() call in main.c explaining the 
> problems.  would having the call in main() be better - turn it off at 
> the earliest possible moment?

I just re-read the man page, it doesn't matter where it's disabled.  The
gmon.out file is going to be overwritten each time gdb is run no matter
what.

If you're profiling something that takes real CPU, the start-up functions
to get to control_main are background noise.  

> gdb is pure iso c, the params forward decl isn't needed.  you may want 
> to look at http://sources.redhat.com/gdb/ari/ for a check list of things 
> recently zapped from gdb.

The forward decl in maint.c?  Tom was following the convention of the
other functions in that file..

Jason

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Mon, 10 Sep 2001 11:52:45 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> 
> > i think the command should always be present.  looking at the patch, it 
> > appears to have started out that way.
> 
> I'd disagree.  First, these functions are not very portable.  Our
> very own RH 6.2 box that is sourceware.cygnus.com does not have
> moncontrol() or monstart(), and -pg doesn't seem to be usable at
> all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
> fine.

We could autoconfiscate this, couldn't we?

> I just re-read the man page, it doesn't matter where it's disabled.  The
> gmon.out file is going to be overwritten each time gdb is run no matter
> what.

Not if we use monstartup: the file is not touched unless monstartup is
called.

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Mon, Sep 10, 2001 at 11:40:38PM +0300, Eli Zaretskii wrote:

> > I'd disagree.  First, these functions are not very portable.  Our
> > very own RH 6.2 box that is sourceware.cygnus.com does not have
> > moncontrol() or monstart(), and -pg doesn't seem to be usable at
> > all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
> > fine.
> 
> We could autoconfiscate this, couldn't we?

We could detect whether moncontrol() exists or not, but there aren't
any macros specifically related to profiling in automake/autoconf.

I suppose we could have a test that would try to include a call to
moncontrol() and see if it causes a link-time error when not paired
with a -pg.  I don't see the point -- without compiling the program
with -pg, running moncontrol() is pointless.  You're going to have
an ifdef in main.c either way - either to check ENABLE_PROFILING
or check HAVE_MONCONTROL - and profiling is not going to work unless
you're specifically compiling gdb with profiling in mind.

> > I just re-read the man page, it doesn't matter where it's disabled.  The
> > gmon.out file is going to be overwritten each time gdb is run no matter
> > what.
> 
> Not if we use monstartup: the file is not touched unless monstartup is
> called.

monstartup provides some information, but not everything that full
profiling gets you.  I think we should stick with linking -pg and
using moncontrol().

Jason

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

On Mon, 10 Sep 2001, Jason Molenda wrote:

> > > I'd disagree.  First, these functions are not very portable.  Our
> > > very own RH 6.2 box that is sourceware.cygnus.com does not have
> > > moncontrol() or monstart(), and -pg doesn't seem to be usable at
> > > all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
> > > fine.
> > 
> > We could autoconfiscate this, couldn't we?
> 
> We could detect whether moncontrol() exists or not, but there aren't
> any macros specifically related to profiling in automake/autoconf.

There's a general-purpose macro to test for an existence of a
function; I think that is all we need.

> You're going to have
> an ifdef in main.c either way - either to check ENABLE_PROFILING
> or check HAVE_MONCONTROL - and profiling is not going to work unless
> you're specifically compiling gdb with profiling in mind.

The HAVE_* macros are the ``normal'' way these things should work, I
think.

> > > I just re-read the man page, it doesn't matter where it's disabled.  The
> > > gmon.out file is going to be overwritten each time gdb is run no matter
> > > what.
> > 
> > Not if we use monstartup: the file is not touched unless monstartup is
> > called.
> 
> monstartup provides some information, but not everything that full
> profiling gets you.

Sorry, I don't understand what you mean.  AFAIK, a program linked with
"-pg" calls monstartup in its startup code.  So using monstartup
manually where you want to start profiling should not lose any
information, while avoiding the noise in the profile from GDB's
startup code and gratuitous overwriting of gmon.out.

Re: Missing routines: gdb/top.c

[John David Anglin]

The module top.c seems to have been corrupted in the most recent
cvs release (19990614).  It appears to be missing procedures such
as init_main.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6605)
From jsm@cygnus.com Tue Jun 15 23:18:00 1999
From: Jason Molenda <jsm@cygnus.com>
To: John David Anglin <dave@hiauly1.hia.nrc.ca>
Cc: gdb-patches@sourceware.cygnus.com, gdb-testers@sourceware.cygnus.com
Subject: 1999-06-14 snapshot is missing part of top.c (was: Missing routines: gdb/top.c)
Date: Tue, 15 Jun 1999 23:18:00 -0000
Message-id: <19990615231757.A5630@cygnus.com>
References: <no.id> <199906160309.XAA04263@hiauly1.hia.nrc.ca> <199906160309.XAA04263@hiauly1.hia.nrc.ca>
X-SW-Source: 1999-q2/msg00128.html
Content-length: 492

On Tue, Jun 15, 1999 at 11:09:58PM -0400, John David Anglin wrote:
> 
> The module top.c seems to have been corrupted in the most recent
> cvs release (19990614).  It appears to be missing procedures such
> as init_main.

Well, that was a silly mistake.

I've attached a patch that includes the missing lines of gdb/top.c.
The next snapshot will be correct.c.  I'm not going to fix the CVS
repository, it'll get fixed next Monday when I import the next snapshot.

Thanks for the note,

Jason
From jimb@cygnus.com Thu Jun 17 13:18:00 1999
From: Jim Blandy <jimb@cygnus.com>
To: gdb-patches@sourceware.cygnus.com
Subject: patch: parsing Java characters
Date: Thu, 17 Jun 1999 13:18:00 -0000
Message-id: <199906172018.PAA14268@zwingli.cygnus.com>
X-SW-Source: 1999-q2/msg00129.html
Content-length: 7509

This is about to be committed.

1999-06-04  Jim Blandy	<jimb@savonarola.red-bean.com>

	Make the '/c' print format use a true character type.  This is
	more appropriate than builtin_type_char for languages other than
	C, and C tolerates it.
	* gdbtypes.c (builtin_type_true_char): New variable.
	(build_gdbtypes): Initialize it.
	* gdbtypes.h (builtin_type_true_char): New declaration.
	* printcmd.c (print_scalar_formatted): When the format is 'c',
	extract the value as a builtin_type_true_char.

Index: gdb/gdbtypes.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/gdbtypes.c,v
retrieving revision 2.88.12.1
diff -c -c -2 -p -r2.88.12.1 gdbtypes.c
*** gdb/gdbtypes.c	1999/03/26 01:06:29	2.88.12.1
--- gdb/gdbtypes.c	1999/06/11 04:21:10
*************** Foundation, Inc., 59 Temple Place - Suit
*** 39,42 ****
--- 39,43 ----
  struct type *builtin_type_void;
  struct type *builtin_type_char;
+ struct type *builtin_type_true_char;
  struct type *builtin_type_short;
  struct type *builtin_type_int;
*************** build_gdbtypes ()
*** 2785,2789 ****
  	       "char", (struct objfile *) NULL);
    TYPE_FLAGS (builtin_type_char) |= TYPE_FLAG_NOSIGN;
!   
    builtin_type_signed_char =
      init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
--- 2786,2793 ----
  	       "char", (struct objfile *) NULL);
    TYPE_FLAGS (builtin_type_char) |= TYPE_FLAG_NOSIGN;
!   builtin_type_true_char = 
!     init_type (TYPE_CODE_CHAR, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
! 	       0,
! 	       "true character", (struct objfile *) NULL);
    builtin_type_signed_char =
      init_type (TYPE_CODE_INT, TARGET_CHAR_BIT / TARGET_CHAR_BIT,
Index: gdb/gdbtypes.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/gdbtypes.h,v
retrieving revision 2.71.12.1
diff -c -c -2 -p -r2.71.12.1 gdbtypes.h
*** gdb/gdbtypes.h	1999/03/26 01:06:30	2.71.12.1
--- gdb/gdbtypes.h	1999/06/11 04:21:11
*************** extern struct type *builtin_type_int64;
*** 845,848 ****
--- 845,853 ----
  extern struct type *builtin_type_uint64;
  
+ /* We use this for the '/c' print format, because builtin_type_char is
+    just a one-byte integral type, which languages less laid back than
+    C will print as ... well, a one-byte integral type.  */
+ extern struct type *builtin_type_true_char;
+ 
  /* This type represents a type that was unrecognized in symbol
     read-in.  */
Index: gdb/printcmd.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/printcmd.c,v
retrieving revision 1.148
diff -c -c -2 -p -r1.148 printcmd.c
*** gdb/printcmd.c	1999/03/10 00:14:04	1.148
--- gdb/printcmd.c	1999/06/11 04:21:12
*************** print_scalar_formatted (valaddr, type, f
*** 443,448 ****
  
      case 'c':
!       value_print (value_from_longest (builtin_type_char, val_long), stream, 0,
! 		   Val_pretty_default);
        break;
  
--- 443,448 ----
  
      case 'c':
!       value_print (value_from_longest (builtin_type_true_char, val_long),
! 		   stream, 0, Val_pretty_default);
        break;
  
Index: gdb/testsuite/gdb.java/jv-print.exp
===================================================================
RCS file: jv-print.exp
diff -N jv-print.exp
*** gdb/testsuite/gdb.java/jv-print.exp	Mon Dec 31 20:00:00 1979
--- gdb/testsuite/gdb.java/jv-print.exp	Thu Jun 10 22:00:25 1999
***************
*** 0 ****
--- 1,141 ----
+ # Copyright (C) 1999 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 2 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, write to the Free Software
+ # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+ 
+ # Please email any bugs, comments, and/or additions to this file to:
+ # bug-gdb@prep.ai.mit.edu
+ 
+ if $tracelevel then {
+ 	strace $tracelevel
+ }
+ 
+ set prms_id 0
+ set bug_id 0
+ 
+ # Set the current language to Java.  This counts as a test.  If it
+ # fails, then we skip the other tests.
+ 
+ proc set_lang_java {} {
+     global gdb_prompt
+ 
+     if [gdb_test "set language java" "" "set language java"] {
+ 	return 0
+     }
+ 
+     if [gdb_test "show language" ".* source language is \"java\".*"] {
+ 	return 0
+     }
+     return 1;
+ }
+ 
+ proc test_integer_literals_accepted {} {
+     global gdb_prompt
+ 
+     # Test various decimal values.
+ 
+     gdb_test "p 123" " = 123"
+     gdb_test "p -123" " = -123"
+     gdb_test "p/d 123" " = 123"
+ 
+     # Test various octal values.
+ 
+     gdb_test "p 0123" " = 83"
+     gdb_test "p 00123" " = 83"
+     gdb_test "p -0123" " = -83"
+     gdb_test "p/o 0123" " = 0123"
+ 
+     # Test various hexadecimal values.
+ 
+     gdb_test "p 0x123" " = 291"
+     gdb_test "p -0x123" " = -291"
+     gdb_test "p 0x0123" " = 291"
+     gdb_test "p -0x0123" " = -291"
+     gdb_test "p 0xABCDEF" " = 11259375"
+     gdb_test "p 0xabcdef" " = 11259375"
+     gdb_test "p 0xAbCdEf" " = 11259375"
+     gdb_test "p/x 0x123" " = 0x123"
+ }
+ 
+ proc test_character_literals_accepted {} {
+     global gdb_prompt
+ 
+     gdb_test "p 'a'" " = 'a'"
+     gdb_test "p/c 'a'" " = 'a'"
+     gdb_test "p/c 70" " = 'F'"
+     gdb_test "p/x 'a'" " = 0x61"
+     gdb_test "p/d 'a'" " = 97"
+     gdb_test "p/t 'a'" " = 1100001"
+     gdb_test "p/x '\\377'" " = 0xff"
+     gdb_test "p '\\''" " = '\\\\''"
+     # Note "p '\\'" => "= 92 '\\'"
+     gdb_test "p '\\\\'" " = '\\\\\\\\'"
+ 
+     # Test the /c print format.
+ }
+ 
+ proc test_integer_literals_rejected {} {
+     global gdb_prompt
+ 
+     test_print_reject "p 0x" 
+     gdb_test "p ''" "Empty character constant\\."
+     gdb_test "p '''" "Empty character constant\\."
+     test_print_reject "p '\\'"
+ 
+     # Note that this turns into "p '\\\'" at gdb's input.
+     test_print_reject "p '\\\\\\'"
+ 
+     # Test various decimal values.
+ 
+     test_print_reject "p DEADBEEF"
+ 
+     test_print_reject "p 123DEADBEEF"
+     test_print_reject "p 123foobar.bazfoo3"
+     test_print_reject "p 123EEEEEEEEEEEEEEEEE33333k333"
+     gdb_test "p 123.4+56.7" "180.(099\[0-9]*|100\[0-9\]*)" "check for floating addition"
+ 
+     # Test various octal values.
+ 
+     test_print_reject "p 09" 
+     test_print_reject "p 079" 
+ 
+     # Test various hexadecimal values.
+ 
+     test_print_reject "p 0xG" 
+     test_print_reject "p 0xAG" 
+ }
+ 
+ 
+ 
+ # Start with a fresh gdb.
+ 
+ gdb_exit
+ gdb_start
+ gdb_reinitialize_dir $srcdir/$subdir
+ 
+ gdb_test "print \$pc" "No registers\\."
+ # FIXME: should also test "print $pc" when there is an execfile but no
+ # remote debugging target, process or corefile.
+ 
+ gdb_test "set print sevenbit-strings" ""
+ gdb_test "set print address off" "" ""
+ gdb_test "set width 0" ""
+ 
+ if [set_lang_java] then {
+     test_integer_literals_accepted
+     test_character_literals_accepted
+     test_integer_literals_rejected
+ } else {
+     fail "Java print command tests suppressed"
+ }

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Tue, Sep 11, 2001 at 09:39:16AM +0200, Eli Zaretskii wrote:

> > monstartup provides some information, but not everything that full
> > profiling gets you.
> 
> Sorry, I don't understand what you mean.  AFAIK, a program linked with
> "-pg" calls monstartup in its startup code.  

No, there is a lot more to a profiled program than that.  The
compiler adds instrumentation in the code, sometimes simple calls
to mcount, sometimes more than that.  It'll link in different
startup files (e.g. "gcrt1", "gcrtend"), and different versions of
libc (e.g. "c_p"), as well as the "gmon" library on some systems.
A simple call to monstartup() won't do that.  The man page on a
FreeBSD system for monstartup() includes this text:

     Programs that are not loaded with -pg may selectively collect profiling
     statistics by calling monstartup() with the range of addresses to be pro-
     filed.  
[...]
     Only functions in that range that have been compiled with the -pg
     option to cc(1) will appear in the call graph part of the output; howev-
     er, all functions in that address range will have their execution time
     measured.  

Calling monstartup() will get you the timing information, but not
the call graph information on a FreeBSD system, apparently.  On a
Linux system, calling monstartup() doesn't get you anything unless
you compile with -pg.


The point of this is that profiling is a compile-time decision.
If you're not compiling for profiling, then there's little reason to
include calls to the profiling control functions.  

I know it seems strange to ifdef the call to moncontrol() like
Tom's patch does, but I don't see any gain to using an autoconf
conditional -- the moncontrol() call is only going to work when
you're compiling specifically for profiling.  

Think about how the code would look.  A test to see if moncontrol()
works without -pg is not useful -- if we're on a system where
moncontrol() doesn't work without -pg, then we wouldn't set
HAVE_MONCONTROL (or whatever the macro setting is), but we _would_
be compiling -pg so we _would_ have moncontrol().  If we're adding a
test to see if moncontrol is available when -pg are used, then we
couldn't use the results of that in a non-pg gdb.  The code which reads

#ifdef ENABLE_PROFILE
    moncontrol (0);
#endif

Would now read

#ifdef ENABLE_PROFILE
#ifdef HAVE_MONCONTROL
    moncontrol (0);
#endif
#endif

Unless we find some system which has profiling but doesn't have
moncontrol(), I don't see any benefit to using autoconf to detect
this.

Jason

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

On Tue, 11 Sep 2001, Jason Molenda wrote:

> On Tue, Sep 11, 2001 at 09:39:16AM +0200, Eli Zaretskii wrote:
> 
> > > monstartup provides some information, but not everything that full
> > > profiling gets you.
> > 
> > Sorry, I don't understand what you mean.  AFAIK, a program linked with
> > "-pg" calls monstartup in its startup code.  
> 
> No, there is a lot more to a profiled program than that.  The
> compiler adds instrumentation in the code, sometimes simple calls
> to mcount, sometimes more than that.  It'll link in different
> startup files (e.g. "gcrt1", "gcrtend"), and different versions of
> libc (e.g. "c_p"), as well as the "gmon" library on some systems.
> A simple call to monstartup() won't do that.

Granted, I already know that ;-)

What I meant is that given you did compile with -pg, monstartup should do 
the same as linking with -pg, with the possible exception of linking in 
libc_p.a.  (I don't think profiling the library is something you would 
want to do in this case.  IIRC, many systems don't even have libc_p 
installed.)

>      Programs that are not loaded with -pg may selectively collect profiling
>      statistics by calling monstartup() with the range of addresses to be pro-
>      filed.  
> [...]
>      Only functions in that range that have been compiled with the -pg
>      option to cc(1) will appear in the call graph part of the output; howev-
>      er, all functions in that address range will have their execution time
>      measured.  
> 
> Calling monstartup() will get you the timing information, but not
> the call graph information on a FreeBSD system, apparently.  On a
> Linux system, calling monstartup() doesn't get you anything unless
> you compile with -pg.

I think there's a misunderstanding: I know that you need to compile
with -pg, I was talking about _linking_ with -pg (which is what causes 
the profiling to start automatically, and requires that you stop it
in GDB's `main').

> #ifdef ENABLE_PROFILE
> #ifdef HAVE_MONCONTROL
>     moncontrol (0);
> #endif
> #endif
> 
> Unless we find some system which has profiling but doesn't have
> moncontrol(), I don't see any benefit to using autoconf to detect
> this.

I know at least one such system: DJGPP (moncontrol and monstartup were 
added to CVS lately, but they are not in the last released version).

Does Cygwin have these functions?

Anyway, I thought it was a policy in GNU projects to test for presence of 
any non-Posix function that a program needs.

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Tue, Sep 11, 2001 at 10:57:34AM +0200, Eli Zaretskii wrote:

> What I meant is that given you did compile with -pg, monstartup should do 
> the same as linking with -pg, with the possible exception of linking in 
> libc_p.a.  (I don't think profiling the library is something you would 
> want to do in this case.  IIRC, many systems don't even have libc_p 
> installed.)

Are you suggesting that a program linked with -pg should call monstartup()?
There isn't any point - you're already profiling when you hit main() in a
program compiled -pg.

> Anyway, I thought it was a policy in GNU projects to test for presence of 
> any non-Posix function that a program needs.

I could add a check for moncontrol(), but obviously the check will
have to be run with -pg or it will be useless (cf my last note).
A failure could indicate either a lack of -pg support, or a lack
of the moncontrol() function.

But remember, the only time _any_ of this is going to be run is
when a developer has specifically configured their tree with
--enable-profiling.  And the code in main.c is inevitably going
to look like

#ifdef ENABLE_PROFILING
#ifdef HAVE_MONCONTROL
    moncontrol (0);
#endif
#endif

(or a compound #if, whatever.)  I really don't see much point to
bothering wiith a check for moncontrol.  I suppose it means a gdb
developer who tries to enable profiling without the necessary
moncontrol() function will find out about his folly at configure-time
instead of link-time, but that's the only benefit I can see.


I'm not trying to be recalcitrant (it comes naturally :-); if people
think that it's worthwhile to add an autoconf check for moncontrol,
I'll add it in there.  But the results of this check will be ignored
99.9% of the time (because the vast majority of trees are not going
to be configured --enable-profiling).

Jason

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Tue, 11 Sep 2001 23:59:55 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> 
> On Tue, Sep 11, 2001 at 10:57:34AM +0200, Eli Zaretskii wrote:
> 
> > What I meant is that given you did compile with -pg, monstartup should do 
> > the same as linking with -pg, with the possible exception of linking in 
> > libc_p.a.  (I don't think profiling the library is something you would 
> > want to do in this case.  IIRC, many systems don't even have libc_p 
> > installed.)
> 
> Are you suggesting that a program linked with -pg should call monstartup()?

No, I mean that a program _compiled_ with -pg, but linked _without_ -pg
will not start profiling until it calls monstartup.  When you _link_
with -pg, you link into your program a module, normally called gcrt0.o,
which calls monstartup (or its equivalent) right from the beginning.

> There isn't any point - you're already profiling when you hit main() in a
> program compiled -pg.

Program compiled with -pg, but linked without -pg will not
automatically profile itself, AFAIK.

> #ifdef ENABLE_PROFILING
> #ifdef HAVE_MONCONTROL
>     moncontrol (0);
> #endif
> #endif
> 
> (or a compound #if, whatever.)  I really don't see much point to
> bothering wiith a check for moncontrol.  I suppose it means a gdb
> developer who tries to enable profiling without the necessary
> moncontrol() function will find out about his folly at configure-time
> instead of link-time, but that's the only benefit I can see.

I think this benefit should not be disposed off too easily.  A person
who tries to configure with --enable-profiling might not realize we
require moncontrol, since usually building a profiling version of a
program doesn't require moncontrol, just -pg.

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Wed, Sep 12, 2001 at 03:56:46PM +0300, Eli Zaretskii wrote:

> No, I mean that a program _compiled_ with -pg, but linked _without_ -pg
> will not start profiling until it calls monstartup.  

Ah, OK.  I knew we were talking about different things somehow.

Don't you think we're going to get real portability headaches this
way? In some cases the mcount et al functions are provided by the
compiler, in some they might be in libc, in some they might be in
a profiling version of libc or in a separate gmon library altogether.

I think the gprof designers on all the systems out there would assume
that if you're compiling with -pg, you're linking with -pg.  Doing
it differently could easily mess up their assumptions.

All of this with monstartup() is only to avoid overwriting the
gmon.out file whenever you start a profiling gdb.  I don't think
it's worth the trouble.  A programmer who is compiling with
--enable-profiling is presumably clever enough to understand that
gmon.out must be moved aside if they don't want it overwritten.
Or he'll learn after the first time he has it overwritten.

If we were to add these monstartup() calls, we'd also have the
portability problems that I mentioned earlier.  Not all systems
that provide profiling and moncontrol() provide the monstartup()
call.  So we'd need an autoconf test to detect that, and we'd define
LD_FLAGS and a C preprocessor macros depending on the results of
that test.  And the documentation for all of this would be confusing.
"Your gmon.out file will be overwritten each time you execute gdb,
unless your system provides the monstartup() system call."




> I really don't see much point to
> > bothering wiith a check for moncontrol.  I suppose it means a gdb
> > developer who tries to enable profiling without the necessary
> > moncontrol() function will find out about his folly at configure-time
> > instead of link-time, but that's the only benefit I can see.
> 
> I think this benefit should not be disposed off too easily.  

OK, I'll add a check for moncontrol and repost the patch in the next day
or so.

J

Re: [RFA] patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Wed, 12 Sep 2001 07:41:41 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> 
> Don't you think we're going to get real portability headaches this
> way? In some cases the mcount et al functions are provided by the
> compiler, in some they might be in libc, in some they might be in
> a profiling version of libc or in a separate gmon library altogether.
> 
> I think the gprof designers on all the systems out there would assume
> that if you're compiling with -pg, you're linking with -pg.  Doing
> it differently could easily mess up their assumptions.

I think that a system which has monstartup should support the method I
described.  Otherwise, monstartup doesn't make much sense.

> All of this with monstartup() is only to avoid overwriting the
> gmon.out file whenever you start a profiling gdb.

I'm mostly worried about getting noise into the profile, due to
profiling that starts as soon as GDB starts running, until you turn it
off with moncontrol.  That noise might hide interesting details, and
might trick a developer to dismiss important findings as noise.

> If we were to add these monstartup() calls, we'd also have the
> portability problems that I mentioned earlier.  Not all systems
> that provide profiling and moncontrol() provide the monstartup()
> call.  So we'd need an autoconf test to detect that, and we'd define
> LD_FLAGS and a C preprocessor macros depending on the results of
> that test.  And the documentation for all of this would be confusing.
> "Your gmon.out file will be overwritten each time you execute gdb,
> unless your system provides the monstartup() system call."

Somehow, I don't see these problems to be as grave as you do ;-)

I think the benefits justify this.  But that's just me...

Re: [RFA] patch to add 'maint profile-gdb' command

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@is.elta.co.il> writes:

Eli> I'm mostly worried about getting noise into the profile, due to
Eli> profiling that starts as soon as GDB starts running, until you
Eli> turn it off with moncontrol.  That noise might hide interesting
Eli> details, and might trick a developer to dismiss important
Eli> findings as noise.

moncontrol(0) is called as the second thing in main().  That's pretty
early.  My experience with this patch from when I wrote it is that the
startup noise is ignorable.  If it shows up anywhere near the top of
your call graph then you probably don't have a real performance
problem to worry about.

Tom

Re: [RFA] patch to add 'maint profile-gdb' command

[Jason Molenda]

On Wed, Sep 12, 2001 at 01:11:42PM -0600, Tom Tromey wrote:

> moncontrol(0) is called as the second thing in main().  That's pretty
> early.  My experience with this patch from when I wrote it is that the
> startup noise is ignorable.  

I'd second that.  I just tried starting an --enable-profiling gdb
and exiting without running "maint profile-gdb on", and I got this
in the "flat profile" section of gprof's output:

  %   cumulative   self              self     total           
 time   seconds   seconds    calls  ms/call  ms/call  name    
  0.0       0.00     0.00        1     0.00     0.00  _captured_main [26]
  0.0       0.00     0.00        1     0.00     0.00  _catch_errors [27]
  0.0       0.00     0.00        1     0.00     0.00  _main [28]
  0.0       0.00     0.00        1     0.00     0.00  _save_cleanups [29]
  0.0       0.00     0.00        1     0.00     0.00  _save_my_cleanups [30]

For comparison, yesterday I was profiling the behavior of a single
'next' under MI and the functions being called the most
(lookup_minimal_symbol_by_pc_section_from_objfile and friends) were
being called thousands of times.

Jason

[patch] Don't set DT_HP_DEBUG_PRIVATE in solib-pa64.c

[John David Anglin]

Hi,

> One thing that I have noticed is that the shared libraries are always
> forced private which makes them writeable.  I recently had a program
> which dropped core when the library code was shared but not when they
> were private.

The patch below implements my suggestion to not set DT_HP_DEBUG_PRIVATE
in solib-pa64.c as the user can do this with chatr (or pxdb).  Tested
on hppa2.0w-hp-hpux11.11.

Ok?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-08-06  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* solib-pa64.c (pa64_solib_create_inferior_hook): Don't set
	DT_HP_DEBUG_PRIVATE.  Add warning if DT_HP_DEBUG_PRIVATE is not set.
	Revise comment.
	(pa64_current_sos): Remove map private warning warning.

Index: solib-pa64.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-pa64.c,v
retrieving revision 1.10
diff -u -3 -p -r1.10 solib-pa64.c
--- solib-pa64.c	1 Jan 2008 22:53:13 -0000	1.10
+++ solib-pa64.c	6 Aug 2008 19:13:08 -0000
@@ -319,13 +322,12 @@ bfd_lookup_symbol (bfd *abfd, char *symn
    to tell the dynamic linker that a private copy of the library is
    needed (so GDB can set breakpoints in the library).
 
-   We need to set two flag bits in this routine.
-
-     DT_HP_DEBUG_PRIVATE to indicate that shared libraries should be
-     mapped private.
-
-     DT_HP_DEBUG_CALLBACK to indicate that we want the dynamic linker to
-     call the breakpoint routine for significant events.  */
+   We need to set DT_HP_DEBUG_CALLBACK to indicate that we want the
+   dynamic linker to call the breakpoint routine for significant events.
+   We used to set DT_HP_DEBUG_PRIVATE to indicate that shared libraries
+   should be mapped private.  However, this flag can be set using
+   "chatr +dbg enable".  Not setting DT_HP_DEBUG_PRIVATE allows debugging
+   with shared libraries mapped shareable.  */
 
 static void
 pa64_solib_create_inferior_hook (void)
@@ -357,8 +359,13 @@ pa64_solib_create_inferior_hook (void)
   if (! read_dynamic_info (shlib_info, &dld_cache))
     error (_("Unable to read the .dynamic section."));
 
+  /* If the libraries were not mapped private, warn the user.  */
+  if ((dld_cache.dld_flags & DT_HP_DEBUG_PRIVATE) == 0)
+    warning (_("The shared libraries were not privately mapped; setting a\n"
+    	     "breakpoint in a shared library will not work. Use \"chatr +dbg "
+	     "enable\"\nto map shared library text privately.\n"));
+
   /* Turn on the flags we care about.  */
-  dld_cache.dld_flags |= DT_HP_DEBUG_PRIVATE;
   dld_cache.dld_flags |= DT_HP_DEBUG_CALLBACK;
   status = target_write_memory (dld_cache.dld_flags_addr,
 				(char *) &dld_cache.dld_flags,
@@ -451,12 +458,6 @@ pa64_current_sos (void)
     if (! read_dld_descriptor ())
       return NULL;
 
-  /* If the libraries were not mapped private, warn the user.  */
-  if ((dld_cache.dld_flags & DT_HP_DEBUG_PRIVATE) == 0)
-    warning (_("The shared libraries were not privately mapped; setting a\n"
-    	     "breakpoint in a shared library will not work until you rerun "
-	     "the program.\n"));
-
   for (dll_index = -1; ; dll_index++)
     {
       struct load_module_desc dll_desc;

Re: [patch] Don't set DT_HP_DEBUG_PRIVATE in solib-pa64.c

[Mark Kettenis]

> Date: Wed, 6 Aug 2008 15:24:12 -0400 (EDT)
> From: "John David Anglin" <dave@hiauly1.hia.nrc.ca>
> 
> Hi,
> 
> > One thing that I have noticed is that the shared libraries are always
> > forced private which makes them writeable.  I recently had a program
> > which dropped core when the library code was shared but not when they
> > were private.
> 
> The patch below implements my suggestion to not set DT_HP_DEBUG_PRIVATE
> in solib-pa64.c as the user can do this with chatr (or pxdb).  Tested
> on hppa2.0w-hp-hpux11.11.

Do you run chatr on the executable or on the shared library to do this?

If you need to change the shared library, then I don't think this is
the right thing to do since the user may not have write permission to
do that.

Otherwise, I think this is ok (but perhaps the message that suggests
using chatr can be improved).

Mark

Re: [patch] Don't set DT_HP_DEBUG_PRIVATE in solib-pa64.c

[John David Anglin]

> Do you run chatr on the executable or on the shared library to do this?

Running chatr on the executable with "+dbg enable" causes all the shared
libraries used by the executable to be mapped private (except for the dynamic
loader).  I've not tried it on a shared library but it looks like from
the documentation and chatr's output that this would allow private mapping
for a specific set of libraries.

> If you need to change the shared library, then I don't think this is
> the right thing to do since the user may not have write permission to
> do that.

The normal situation is to do it on the executable.  If a user wanted
to use "+dbg enable" on a system library, I believe it would be possible
to copy the library and use LD_LIBRARY_PATH to select the copy.  In the
32-bit runtime, this is tricker since library paths are hardcoded.

> Otherwise, I think this is ok (but perhaps the message that suggests
> using chatr can be improved).

I'll think about that.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [patch] Don't set DT_HP_DEBUG_PRIVATE in solib-pa64.c

[John David Anglin]

> Otherwise, I think this is ok (but perhaps the message that suggests
> using chatr can be improved).

How does this look?

The best online documentation for the methods to privately map shared
library text is <http://docs.hp.com/en/B2355-60130/chatr_pa.1.html>.
This is for 11i v3 but the same features are available on all HP-UX 11
versions with a recent linker patch.  On HP-UX 11.11, chatr +dbg
support was added in PHSS_32864.  Using _HP_DLDOPTS, it is possible
to map privately all or specific libraries without modifying the
libraries or executable.

It is also possible to use "pxdb -s on" to privately map the text segments
of shared libraries.  See <http://docs.hp.com/en/B2355-60130/ld_pa.1.html>.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-08-06  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* solib-pa64.c (pa64_solib_create_inferior_hook): Don't set
	DT_HP_DEBUG_PRIVATE.  Add warning if DT_HP_DEBUG_PRIVATE is not set.
	Revise comment.
	(pa64_current_sos): Remove map private warning warning.

Index: solib-pa64.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-pa64.c,v
retrieving revision 1.12
diff -u -3 -p -r1.12 solib-pa64.c
--- solib-pa64.c	6 Aug 2008 20:17:21 -0000	1.12
+++ solib-pa64.c	6 Aug 2008 22:49:28 -0000
@@ -322,13 +322,12 @@ bfd_lookup_symbol (bfd *abfd, char *symn
    to tell the dynamic linker that a private copy of the library is
    needed (so GDB can set breakpoints in the library).
 
-   We need to set two flag bits in this routine.
-
-     DT_HP_DEBUG_PRIVATE to indicate that shared libraries should be
-     mapped private.
-
-     DT_HP_DEBUG_CALLBACK to indicate that we want the dynamic linker to
-     call the breakpoint routine for significant events.  */
+   We need to set DT_HP_DEBUG_CALLBACK to indicate that we want the
+   dynamic linker to call the breakpoint routine for significant events.
+   We used to set DT_HP_DEBUG_PRIVATE to indicate that shared libraries
+   should be mapped private.  However, this flag can be set using
+   "chatr +dbg enable".  Not setting DT_HP_DEBUG_PRIVATE allows debugging
+   with shared libraries mapped shareable.  */
 
 static void
 pa64_solib_create_inferior_hook (void)
@@ -360,8 +359,15 @@ pa64_solib_create_inferior_hook (void)
   if (! read_dynamic_info (shlib_info, &dld_cache))
     error (_("Unable to read the .dynamic section."));
 
+  /* If the libraries were not mapped private, warn the user.  */
+  if ((dld_cache.dld_flags & DT_HP_DEBUG_PRIVATE) == 0)
+    warning
+      (_("Private mapping of shared library text was not specified\n"
+	 "by the executable; setting a breakpoint in a shared library which\n"
+	 "is not privately mapped will not work.  See the HP-UX 11i v3 chatr\n"
+	 "manpage for methods to privately map shared library text.\n"));
+
   /* Turn on the flags we care about.  */
-  dld_cache.dld_flags |= DT_HP_DEBUG_PRIVATE;
   dld_cache.dld_flags |= DT_HP_DEBUG_CALLBACK;
   status = target_write_memory (dld_cache.dld_flags_addr,
 				(char *) &dld_cache.dld_flags,
@@ -454,12 +460,6 @@ pa64_current_sos (void)
     if (! read_dld_descriptor ())
       return NULL;
 
-  /* If the libraries were not mapped private, warn the user.  */
-  if ((dld_cache.dld_flags & DT_HP_DEBUG_PRIVATE) == 0)
-    warning (_("The shared libraries were not privately mapped; setting a\n"
-    	     "breakpoint in a shared library will not work until you rerun "
-	     "the program.\n"));
-
   for (dll_index = -1; ; dll_index++)
     {
       struct load_module_desc dll_desc;

[patch] Only force private mapping of shared libraries in solib-som.c on HP-UX 10 and earlier

[John David Anglin]

This is a comparible patch to the solib-pa64.c patch.  This one is
somewhat messier as it has to deal with debugging under HP-UX 10 and 11.
I don't have a perfect solution but I think we only support native
debugging.  Another method to get the hpux release might have been
to parse the target_name string, but it is not guaranteed to contain
the OS release information.

The old warning was not accurate as libraries were always forced private
even on the first run.

As mentioned previously, I need this change because I have a GCC libgomp
testsuite failure that fails when shared libraries are not mapped privately,
and passes when they are mapped privately.

Ok?

Thanks,
Dave

2008-08-07  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* solib-som.c: Include string.h and sys/utsname.h.
	(get_hpux_major_release): New function.
	(som_solib_create_inferior_hook): Read dynamic linker header.  Warn
	about shared library private mapping on HP-UX 11 and later.  Only force
	private mapping of shared libraries on HP-UX 10 and earlier.
	(link_map_start): Delete warning.

Index: solib-som.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-som.c,v
retrieving revision 1.15
diff -u -3 -p -r1.15 solib-som.c
--- solib-som.c	1 Jan 2008 22:53:13 -0000	1.15
+++ solib-som.c	7 Aug 2008 20:03:53 -0000
@@ -30,6 +30,9 @@
 #include "solist.h"
 #include "solib.h"
 
+#include <sys/utsname.h>
+#include <string.h>
+
 #undef SOLIB_SOM_DBG 
 
 /* These ought to be defined in some public interface, but aren't.  They
@@ -125,6 +128,38 @@ som_relocate_section_addresses (struct s
     ;
 }
 
+/* Get HP-UX major release number.  Returns zero if the
+   release is not known.  */
+
+static int
+get_hpux_major_release (void)
+{
+  static int hpux_major_release = -1;
+
+  if (hpux_major_release == -1)
+    {
+      struct utsname x;
+      char *p;
+
+      uname (&x);
+      p = strchr (x.release, '.');
+      hpux_major_release = p ? atoi (p + 1) : 0;
+    }
+
+  return hpux_major_release;
+}
+
+/* DL header flag defines.  */
+#define SHLIB_TEXT_PRIVATE_ENABLE 0x4000
+
+/* The DL header is documented in <shl.h>.  We are only interested
+   in the flags field to determine whether the executable wants shared
+   libraries mapped private.  */
+struct {
+    short junk[37];
+    short flags;
+} dl_header;
+
 /* This hook gets called just before the first instruction in the
    inferior process is executed.
 
@@ -170,6 +205,10 @@ som_solib_create_inferior_hook (void)
   if (bfd_section_size (symfile_objfile->obfd, shlib_info) == 0)
     return;
 
+  /* Read the DL header.  */
+  bfd_get_section_contents (symfile_objfile->obfd, shlib_info,
+			    (char *) &dl_header, 0, sizeof (dl_header));
+
   have_endo = 0;
   /* Slam the pid of the process into __d_pid.
 
@@ -274,8 +313,22 @@ keep_going:
     error (_("Unable to read __dld_flags."));
   dld_flags = extract_unsigned_integer (buf, 4);
 
+  /* If the libraries were not mapped private on HP-UX 11 and later, warn
+     the user.  On HP-UX 10 and earlier, there is no easy way to specify
+     that shared libraries should be privately mapped.  So, we just force
+     private mapping.  */
+  if (get_hpux_major_release () >= 11
+      && (dl_header.flags & SHLIB_TEXT_PRIVATE_ENABLE) == 0
+      && (dld_flags & DLD_FLAGS_MAPPRIVATE) == 0)
+    warning
+      (_("Private mapping of shared library text was not specified\n"
+	 "by the executable; setting a breakpoint in a shared library which\n"
+	 "is not privately mapped will not work.  See the HP-UX 11i v3 chatr\n"
+	 "manpage for methods to privately map shared library text."));
+
   /* Turn on the flags we care about.  */
-  dld_flags |= DLD_FLAGS_MAPPRIVATE;
+  if (get_hpux_major_release () < 11)
+    dld_flags |= DLD_FLAGS_MAPPRIVATE;
   if (have_endo)
     dld_flags |= DLD_FLAGS_HOOKVALID;
   store_unsigned_integer (buf, 4, dld_flags);
@@ -486,12 +539,6 @@ link_map_start (void)
   if ((dld_flags & DLD_FLAGS_LISTVALID) == 0)
     error (_("__dld_list is not valid according to __dld_flags."));
 
-  /* If the libraries were not mapped private, warn the user.  */
-  if ((dld_flags & DLD_FLAGS_MAPPRIVATE) == 0)
-    warning (_("The shared libraries were not privately mapped; setting a\n"
-	     "breakpoint in a shared library will not work until you rerun the "
-	     "program.\n"));
-
   sym = lookup_minimal_symbol ("__dld_list", NULL, NULL);
   if (!sym)
     {

Re: [patch] Only force private mapping of shared libraries in solib-som.c on HP-UX 10 and earlier

[Joel Brobecker]

> 2008-08-07  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
> 
> 	* solib-som.c: Include string.h and sys/utsname.h.
> 	(get_hpux_major_release): New function.
> 	(som_solib_create_inferior_hook): Read dynamic linker header.  Warn
> 	about shared library private mapping on HP-UX 11 and later.  Only force
> 	private mapping of shared libraries on HP-UX 10 and earlier.
> 	(link_map_start): Delete warning.

This seems OK to me - commit at any time. Just a question: Can't we just
force private mapping on HP/UX 11 the same way we do for v10?


-- 
Joel

Re: [patch] Only force private mapping of shared libraries in solib-som.c on HP-UX 10 and earlier

[John David Anglin]

> This seems OK to me - commit at any time. Just a question: Can't we just
> force private mapping on HP/UX 11 the same way we do for v10?

Sure but doing so can change the behavior of a program.  It makes
the text of shared libraries writeable and changes the quadrant where
the libraries are loaded.  So, a bug that appears when shared libraries
are not private can disappear when the libraries are private.  The
vla6.f90 testcase in the libgomp testsuite is a case in point ;)

All HP/UX 11 versions with a fairly recent linker patch have enhanced
support in chatr and the dynamic loader for making shared libraries private.
This makes it easy to force all shared libraries private, or it can be done
on a selective basis.  My previous message about changing solib-pa64.c to
do the same contained links to the HP/UX chatr and ld manpages (11i v3)
that describes this support.

The dl_header has not changed since HP/UX 10.  Thus, it should be possible
to use these features under HP/UX 11 to debug an application built and
linked on HP/UX 10.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [patch] Only force private mapping of shared libraries in solib-som.c on HP-UX 10 and earlier

[Joel Brobecker]

> > This seems OK to me - commit at any time. Just a question: Can't we just
> > force private mapping on HP/UX 11 the same way we do for v10?
> 
> Sure but doing so can change the behavior of a program.

Ah, of course! Your approach make total sense, now.

-- 
Joel

Re: [patch] Only force private mapping of shared libraries in solib-som.c on HP-UX 10 and earlier

[John David Anglin]

> Ah, of course! Your approach make total sense, now.

I commited the solib-som.c and solib-pa64.c changes.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: ttrace: Protocal error

[John David Anglin]

> [New process 20069]
> [New process 20069, lwp 7087826]

[New process 2692, lwp 7130128]
[process 2692, lwp 7130128 exited]

Breakpoint 1, perror_with_name (string=0x0) at ../../src/gdb/utils.c:847
847       err = safe_strerror (errno);
(gdb) frame 1
#1  0x000c9b08 in inf_ttrace_resume_callback (info=0x2319b0, arg=0x7b019048)
    at ../../src/gdb/inf-ttrace.c:813
813             perror_with_name (("ttrace"));
(gdb) p *(struct thread_info *)($r3 - 4)
$21 = {next = 0x4007bc90, ptid = {pid = 2692, lwp = 7130128, tid = 0},
  num = 2, executing_ = 0, state_ = 1, refcount = 0, prev_pc = 0,
  step_resume_breakpoint = 0x0, step_range_start = 0, step_range_end = 0,
  step_frame_id = {stack_addr = 0, code_addr = 0, special_addr = 0,
  stack_addr_p = 0, code_addr_p = 0, special_addr_p = 0}, current_line = 0,
  current_symtab = 0x0, trap_expected = 0, stepping_over_breakpoint = 0,
  stepping_through_solib_after_catch = 0,
  stepping_through_solib_catchpoints = 0x0, continuations = 0x0,
  intermediate_continuations = 0x0, proceed_to_finish = 0,
  step_over_calls = STEP_OVER_NONE, stop_step = 0, step_multi = 0,
  stop_signal = TARGET_SIGNAL_0, stop_bpstat = 0x0, private = 0x40076ac8}
(gdb) p (int)THREAD_EXITED
$22 = 2

Hmmm, the thread seems to have exited but state_ is still 1.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: ttrace: Protocal error

[John David Anglin]

> Hmmm, the thread seems to have exited but state_ is still 1.

The patch below seems to fix the problem.  I was finally able
to catch an abort in vla6.f90.

Ok?

(gdb) r
Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g
warning: Private mapping of shared library text was not specified
by the executable; setting a breakpoint in a shared library which
is not privately mapped will not work.  See the HP-UX 11i v3 chatr
manpage for methods to privately map shared library text.
[New process 6450, lwp 7134092]
[process 6450, lwp 7134092 exited]
[New process 6450, lwp 7134093]
[process 6450, lwp 7134093 exited]
[New process 6450, lwp 7134094]
[process 6450, lwp 7134094 exited]
[New process 6450, lwp 7134095]
[process 6450, lwp 7134095 exited]
[New process 6450, lwp 7134096]
[New process 6450, lwp 7134097]
[New process 6450, lwp 7134098]
[New process 6450, lwp 7134099]
[New process 6450, lwp 7134100]

Program received signal SIGABRT, Aborted.
0xc020da50 in kill () from /usr/lib/libc.2
(gdb) bt
#0  0xc020da50 in kill () from /usr/lib/libc.2
#1  0xc01a7f74 in raise () from /usr/lib/libc.2
#2  0xc01e92c8 in abort_C () from /usr/lib/libc.2
#3  0xc01e9324 in abort () from /usr/lib/libc.2
#4  0xc1c7ea20 in _gfortran_abort ()
    at ../../../gcc/libgfortran/intrinsics/abort.c:39
#5  0x00007d04 in foo.572 (c=<error reading variable>,
    d=<error reading variable>, e=<error reading variable>,
    f=<error reading variable>, g=<error reading variable>,
    h=<error reading variable>, i=<error reading variable>,
    j=<error reading variable>, k=<error reading variable>,
    n=<error reading variable>, _c=<error reading variable>,
    _d=<error reading variable>, _g=<error reading variable>,
    _h=<error reading variable>)
    at /mnt/gnu/gcc/gcc/libgomp/testsuite/libgomp.fortran/vla6.f90:175
#6  0x00007e58 in test.560 ()
    at /mnt/gnu/gcc/gcc/libgomp/testsuite/libgomp.fortran/vla6.f90:189
#7  0x00007ea8 in MAIN__ ()
    at /mnt/gnu/gcc/gcc/libgomp/testsuite/libgomp.fortran/vla6.f90:3
#8  0x00007f30 in main (argc=1, argv=0x7eff050c)
    at ../../../gcc/libgfortran/fmain.c:21
(gdb) info shared
From        To          Syms Read   Shared Object Library
0xc0010000  0xc0044084  Yes         /usr/lib/dld.sl
0xc0e1c000  0xc0e28000  Yes         /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/./libgomp/.libs/libgomp.sl.1
0xc1bc0000  0xc1c9c000  Yes         /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/./libgomp/../libgfortran/.libs/libgfortran.sl.3
0xc0e00000  0xc0e19000  Yes         /mnt/gnu/gcc/objdir/./gcc/libgcc_s.sl
0xc00c0000  0xc00ec000  Yes         /usr/lib/libm.2
0xc03bc000  0xc03c0000  Yes         /usr/lib/librt.2
0xc0050000  0xc0069000  Yes         /opt/langtools/lib/libpthread.1
0xc0100000  0xc024b000  Yes         /usr/lib/libc.2
0xc0005000  0xc0008000  Yes         /usr/lib/libdld.2
0xc0004000  0xc0005000  Yes         /opt/graphics/OpenGL/lib/libogltls.sl

This is with Pedro's change.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-08-08  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* inf-ttrace.c (inf_ttrace_resume_callback): Don't resume dying thread.

Index: inf-ttrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ttrace.c,v
retrieving revision 1.30
diff -u -3 -p -r1.30 inf-ttrace.c
--- inf-ttrace.c	9 Jul 2008 22:23:05 -0000	1.30
+++ inf-ttrace.c	8 Aug 2008 19:57:13 -0000
@@ -787,7 +804,9 @@ inf_ttrace_kill (void)
 static int
 inf_ttrace_resume_callback (struct thread_info *info, void *arg)
 {
-  if (!ptid_equal (info->ptid, inferior_ptid))
+  if (!ptid_equal (info->ptid, inferior_ptid)
+      && !((struct inf_ttrace_private_thread_info *)info->private)->dying
+      && !is_exited (info->ptid))
     {
       pid_t pid = ptid_get_pid (info->ptid);
       lwpid_t lwpid = ptid_get_lwp (info->ptid);
 \f
 

Re: ttrace: Protocal error

[Pedro Alves]

On Friday 08 August 2008 21:14:56, John David Anglin wrote:
> > Hmmm, the thread seems to have exited but state_ is still 1.
>
> The patch below seems to fix the problem.  I was finally able
> to catch an abort in vla6.f90.
>
> Ok?
>
>
>         * inf-ttrace.c (inf_ttrace_resume_callback): Don't resume dying
> thread.
>
> Index: inf-ttrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/inf-ttrace.c,v
> retrieving revision 1.30
> diff -u -3 -p -r1.30 inf-ttrace.c
> --- inf-ttrace.c        9 Jul 2008 22:23:05 -0000       1.30
> +++ inf-ttrace.c        8 Aug 2008 19:57:13 -0000
> @@ -787,7 +804,9 @@ inf_ttrace_kill (void)
>  static int
>  inf_ttrace_resume_callback (struct thread_info *info, void *arg)
>  {
> -  if (!ptid_equal (info->ptid, inferior_ptid))
> +  if (!ptid_equal (info->ptid, inferior_ptid)
> +      && !((struct inf_ttrace_private_thread_info *)info->private)->dying
> +      && !is_exited (info->ptid))
>      {
>        pid_t pid = ptid_get_pid (info->ptid);
>        lwpid_t lwpid = ptid_get_lwp (info->ptid);

From your original backtrace, I'm still puzzled on how you got here.  When
we tag the thread as dying, we return TARGET_WAITKIND_SPURIOUS, which
triggers a resume.  This first resume should have removed the thread
from the thread list.  

If you were really in a TARGET_WAITKIND_SYSCALL_ENTRY, this would be
at least the second resume after the lwp exit.
Maybe I am reading the backtrace wrong though.  If you have the patience,
showing what GDB outputs when you do a "run" after setting
"set debug infrun 1" would help.

Looking again at the TTEVT_LWP_EXIT event, which tags the thread as dying.
When we get it, the lwp is not dead yet:

"TTEVT_LWP_EXIT

    This event flag indicates that the debugger wants to be notified when a 
thread is exiting via the lwp_exit() system call. The thread stops upon entry 
to the system call."

That's why we resume it immediatelly here,

inf_ttrace_wait ()
...
      case TTEVT_LWP_EXIT:
        if (print_thread_events)
          printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (ptid));
        ti = find_thread_pid (ptid);
        gdb_assert (ti != NULL);
        ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
        inf_ttrace_num_lwps--;
(1)     ttrace (TT_LWP_CONTINUE, ptid_get_pid (ptid),
              ptid_get_lwp (ptid), TT_NOPC, 0, 0);
        /* If we don't return -1 here, core GDB will re-add the thread.  */
        ptid = minus_one_ptid;
        break;
...

    /* Make sure all threads within the process are stopped.  */
(2)  if (ttrace (TT_PROC_STOP, tts.tts_pid, 0, 0, 0, 0) == -1)
       perror_with_name (("ttrace"));

    return ptid;
  }


It seems to me, that for some reason, in most cases, the inferior was slow
enough that when you reach (2), the dying thread hadn't exited
yet.  The TT_PROC_STOP call stops all lwps of the process, the
dying one included, I would think.  In that case, you still need the
resume on the dying thread in inf_ttrace_wait.  Otherwise, you *may*
get this bug back, depending on how the OS is waking waiting processes:

 http://www.cygwin.com/ml/gdb-patches/2007-09/msg00238.html

 "So, if the dying thread is stopped, it should be resumed one last time.
 Otherwise, any other thread waiting for its death on pthread_join
 would be blocked forever (e.g. in attachment, a simple program which
 freezes when it is run under GDB)."

That would explain why you trip on this bug, but, Jerome didn't,
I guess.  It may be your testcase triggers the race better.

The way this is solved currently, is also racy.  We
resume all threads, dying threads included, and assume they
will die just shortly, so, we immediately delete dying
threads after resuming:

  if (ptid_equal (ptid, minus_one_ptid))
    {
      /* Let all the other threads run too.  */
      iterate_over_threads (inf_ttrace_resume_callback, NULL);
****  iterate_over_threads (inf_ttrace_delete_dying_threads_callback, NULL);
    }

It could happen, that under stress, GDB handles another event,
and stops all lwps, *before* the dying lwp having a chance
of exiting.  The symptom would again the be one Jerome was
fixing.  Having no idea on how much the thread still executes
after an TTEVT_LWP_EXIT event, I can't tell how likelly this
is to happen, say causing spurious testsuite failures.

The ttrace API doesn't have an "lwp is really gone from
the OS tables" event (sigh), but, it does have
a TTEVT_LWP_CREATE event.  This mean that we can detect
if the OS is reusing an lwpid at that point.

So, to minimise the possible race, how about:

- still try to resume a dying lwp.  Ignore the errno you
  were originally seeing in that case (only).
- on resume failure, delete it from GDBs thread table.
- if by any chance, the lwp exits, and the inferior spawn a
  new lwp, and the OS reuses the same lwpid of the lwp we knew
  was dying, we delete the dying lwp, and add the new one.
  If the OS is reusing the id, the original lwp has to be gone.
  This is just an add_thread call, as that is already handled by it
  internally (*).
- If the thread is still alive, but is dying, let that show
  in "info threads".  The linux pthread support implementation
  also does this.

I think the race below still exists, but it happens to
be innocuous:

- we know an lwp was dying, but, we didn't detect its exit,
  because we resumed it really exited.
- another thread hits breakpoint
- yet another thread spawns a new thread, which reuses the
  dying thread's lwpid.
- GDB is informed of the breakpoint hit.
- Since events are handled sequencially, we will only notice the
  new thread event after handling the breakpoint hit.
- the user resumes the inferior, after inspecting what happened
  at the breakpoint.
- GDB tries to resume the dying lwpid --- actually, this id belongs
  to a new thread by now.  Since we were handling a breakpoint, we
  had stopped the whole process, so this resume succeeds.  We consider
  that the lwp is still dying.
- GDB now handles the TTEVT_LWP_CREATE event, and adds it to GDB's
  thread list, which automatically gets rid of the dying lwp, due
  to PTID collision.

Also, when we detect a TTEVT_LWP_CREATE, TTEVT_LWP_EXIT or 
TTEVT_LWP_TERMINATE, The ttrace docs indicate that only one lwp is
stopped.  There's no reason to stop all lwps, and return
TARGET_WAITKING_SPURIOUS, only to resume all lwps again.
This just adds overhead and messes more with the
scheduling of the inferior than needed.  We could just resume
the stopped lwp, and return TARGET_WAITKIND_IGNORE.  But, we'd not
detect that the thread really is gone until the next whole inferior
resume.  I guess we could also detect if dying threads have already
exited after stopping the whole process and sending signal 0.
We can also do that in inf_ttrace_thread_alive (like inf_ptrace_thread_alive
does), so "info threads" also gets rid of dying threads that have already
really died.

Sorry, that came out longer that I was expecting.  Am I making any sense?
I can work up a patch for this.

If I'm analising the race correctly, an alternative simpler solution
to the above, would be to just ignore a failed resume on a
dying thread, but still try to resume it.  Not resuming dying threads
may be a problem.

-- 
Pedro Alves

Re: ttrace: Protocal error

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 2010 bytes --]

On Sat, 09 Aug 2008, Pedro Alves wrote:

> If I'm analising the race correctly, an alternative simpler solution
> to the above, would be to just ignore a failed resume on a
> dying thread, but still try to resume it.  Not resuming dying threads
> may be a problem.

It occured to me that my suggested change wasn't going to handle the
race associated with threads dying, although it improves the situation.

The ttrace documentation suggested that only stopped threads should
be resumed.  The thread info indicated the thread in question was
running.  I changed the code to only check for thread stopped.  However,
this didn't work.  My testcase hangs when run under gdb:

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 
warning: Private mapping of shared library text was not specified
by the executable; setting a breakpoint in a shared library which
is not privately mapped will not work.  See the HP-UX 11i v3 chatr
manpage for methods to privately map shared library text.
[New process 2316, lwp 7552188]
[process 2316, lwp 7552188 exited]
[New process 2316, lwp 7552189]
[process 2316, lwp 7552189 exited]
[New process 2316, lwp 7552190]
[process 2316, lwp 7552190 exited]
[New process 2316, lwp 7552191]
[process 2316, lwp 7552191 exited]
[New process 2316, lwp 7552192]
[New process 2316, lwp 7552193]
[New process 2316, lwp 7552194]
[New process 2316, lwp 7552195]
[New process 2316, lwp 7552196]

Program received signal SIGINT, Interrupt.
[Switching to process 2316, lwp 7552196]
0xc03bea58 in __lwp_sema_wait () from /usr/lib/librt.2

So, there's apparently a race in detection when a program is stopped.
It looks like we need to try your suggestion(s).

Attached same with debug infrun 1.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

[-- Attachment #2: vla6.dbg --]
[-- Type: text/plain, Size: 4141 bytes --]

(gdb) set debug infrun 1
(gdb) r
Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 
infrun: wait_for_inferior (treat_exec_as_sigtrap=1)
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x91d0
infrun: quietly stopped
infrun: stop_stepping
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: wait_for_inferior (treat_exec_as_sigtrap=1)
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x19f8
infrun: quietly stopped
infrun: stop_stepping
warning: Private mapping of shared library text was not specified
by the executable; setting a breakpoint in a shared library which
is not privately mapped will not work.  See the HP-UX 11i v3 chatr
manpage for methods to privately map shared library text.
infrun: proceed (addr=0xffffffff, signal=0, step=0)
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: wait_for_inferior (treat_exec_as_sigtrap=0)
[New process 2557, lwp 7552464]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[process 2557, lwp 7552464 exited]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552465]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[process 2557, lwp 7552465 exited]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552466]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[process 2557, lwp 7552466 exited]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552467]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[process 2557, lwp 7552467 exited]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x19e0
infrun: BPSTAT_WHAT_CHECK_SHLIBS
infrun: no stepping, continue
infrun: resume (step=1, signal=0), stepping_over_breakpoint=1
infrun: prepare_to_wait
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x19e4
infrun: no stepping, continue
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552468]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552469]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552470]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552471]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[New process 2557, lwp 7552472]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0xc020c878
infrun: context switch
infrun: Switching context from process 2557, lwp 7552460 to process 2557, lwp 7552472
infrun: random signal 2

Program received signal SIGINT, Interrupt.
infrun: stop_stepping
[Switching to process 2557, lwp 7552472]
0xc020c878 in __ksleep () from /usr/lib/libc.2

Re: ttrace: Protocal error

[John David Anglin]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

On Sat, 09 Aug 2008, Pedro Alves wrote:

> If you were really in a TARGET_WAITKIND_SYSCALL_ENTRY, this would be
> at least the second resume after the lwp exit.
> Maybe I am reading the backtrace wrong though.  If you have the patience,
> showing what GDB outputs when you do a "run" after setting
> "set debug infrun 1" would help.

Attached.

Ignoring protocol errors seems to work.  Used the following.  Note
there is a conflict with your change.

Index: inf-ttrace.c
===================================================================
RCS file: /cvs/src/src/gdb/inf-ttrace.c,v
retrieving revision 1.30
diff -u -3 -p -r1.30 inf-ttrace.c
--- inf-ttrace.c	9 Jul 2008 22:23:05 -0000	1.30
+++ inf-ttrace.c	9 Aug 2008 18:35:51 -0000
@@ -787,12 +804,16 @@ inf_ttrace_kill (void)
 static int
 inf_ttrace_resume_callback (struct thread_info *info, void *arg)
 {
-  if (!ptid_equal (info->ptid, inferior_ptid))
+  /* There is a race condition in detecting when a thread is stopped.
+     So, we need to resume "dying" threads and ignore protocol errors
+     that occur from resuming threads that aren't stopped.  */
+  if (!ptid_equal (info->ptid, inferior_ptid) && !is_exited (info->ptid))
     {
       pid_t pid = ptid_get_pid (info->ptid);
       lwpid_t lwpid = ptid_get_lwp (info->ptid);
 
-      if (ttrace (TT_LWP_CONTINUE, pid, lwpid, TT_NOPC, 0, 0) == -1)
+      if (ttrace (TT_LWP_CONTINUE, pid, lwpid, TT_NOPC, 0, 0) == -1
+	  && errno != EPROTO)
 	perror_with_name (("ttrace"));
     }
 
Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

[-- Attachment #2: vla6.dbg.2 --]
[-- Type: text/plain, Size: 1427 bytes --]

(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 
infrun: wait_for_inferior (treat_exec_as_sigtrap=1)
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x91d0
infrun: quietly stopped
infrun: stop_stepping
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: wait_for_inferior (treat_exec_as_sigtrap=1)
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_STOPPED
infrun: stop_pc = 0x19f8
infrun: quietly stopped
infrun: stop_stepping
warning: Private mapping of shared library text was not specified
by the executable; setting a breakpoint in a shared library which
is not privately mapped will not work.  See the HP-UX 11i v3 chatr
manpage for methods to privately map shared library text.
infrun: proceed (addr=0xffffffff, signal=0, step=0)
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: wait_for_inferior (treat_exec_as_sigtrap=0)
[New process 5170, lwp 7674424]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
infrun: prepare_to_wait
[process 5170, lwp 7674424 exited]
infrun: infwait_normal_state
infrun: TARGET_WAITKIND_SPURIOUS
infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
0xc00268dc in ?? ()
ttrace: Protocol error.

Re: ttrace: Protocal error

[Pedro Alves]

On Saturday 09 August 2008 19:48:23, John David Anglin wrote:
> [process 5170, lwp 7674424 exited]
> infrun: infwait_normal_state
> infrun: TARGET_WAITKIND_SPURIOUS
> infrun: resume (step=0, signal=0), stepping_over_breakpoint=0
> 0xc00268dc in ?? ()
> ttrace: Protocol error.

Ok, that clearly shows that's the first resume after a TTEVT_LWP_EXIT.
No TARGET_WAITKIND_SYSCALL_ENTRY involved after all.

Good, I can sleep better now.  ;-)

Thanks,

-- 
Pedro Alves

Re: ttrace: Protocal error

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 2804 bytes --]

On Saturday 09 August 2008 15:51:16, Pedro Alves wrote:


> inf_ttrace_wait ()
> ...
>       case TTEVT_LWP_EXIT:
>         if (print_thread_events)
>           printf_unfiltered (_("[%s exited]\n"), target_pid_to_str (ptid));
>         ti = find_thread_pid (ptid);
>         gdb_assert (ti != NULL);
>         ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
>         inf_ttrace_num_lwps--;
> (1)     ttrace (TT_LWP_CONTINUE, ptid_get_pid (ptid),
>               ptid_get_lwp (ptid), TT_NOPC, 0, 0);
>         /* If we don't return -1 here, core GDB will re-add the thread.  */
>         ptid = minus_one_ptid;
>         break;
> ...
>
>     /* Make sure all threads within the process are stopped.  */
> (2)  if (ttrace (TT_PROC_STOP, tts.tts_pid, 0, 0, 0, 0) == -1)
>        perror_with_name (("ttrace"));
>
>     return ptid;
>   }
>
>
> It seems to me, that for some reason, in most cases, the inferior was slow
> enough that when you reach (2), the dying thread hadn't exited
> yet.  The TT_PROC_STOP call stops all lwps of the process, the
> dying one included, I would think.  In that case, you still need the
> resume on the dying thread in inf_ttrace_wait.  Otherwise, you *may*
> get this bug back, depending on how the OS is waking waiting processes:


> So, to minimise the possible race, how about:
>
> - still try to resume a dying lwp.  Ignore the errno you
>   were originally seeing in that case (only).
> - on resume failure, delete it from GDBs thread table.
> - if by any chance, the lwp exits, and the inferior spawn a
>   new lwp, and the OS reuses the same lwpid of the lwp we knew
>   was dying, we delete the dying lwp, and add the new one.
>   If the OS is reusing the id, the original lwp has to be gone.
>   This is just an add_thread call, as that is already handled by it
>   internally (*).
> - If the thread is still alive, but is dying, let that show
>   in "info threads".  The linux pthread support implementation
>   also does this.

This is what the attached patch does.  In adition to what is
described above, I'm checking if any dying thread is now gone
after stopping the whole process.  I'm checking for lwp "aliveness"
with sending signal 0.  I hope it works as expected against
ttrace stopped threads, otherwise, I'd need another way to detect
if the lwp is still alive.

With this change, we no longer unconditionaly delete the dying 
lwps after the first resume.  This is to prevent that another event
that was already queued is handled and GDB stopping the whole process
before the dying thread having a chance to die.  In this case, we'll
still need another resume in the dying lwp -- until it really exits.

Hope I haven't broken anything badly.  I've never in my live logged in
to an HP-UX system, so wear sunglasses.

-- 
Pedro Alves

[-- Attachment #2: fix_daves_bug.diff --]
[-- Type: text/x-diff, Size: 6398 bytes --]

2008-08-09  Pedro Alves  <pedro@codesourcery.com>

	* inf-ttrace.c: Include <signal.h>
	(inf_ttrace_delete_dead_threads_callback): New.
	(inf_ttrace_resume_lwp): New.
	(inf_ttrace_resume_callback, inf_ttrace_resume): Rewrite.  Don't
	delete dying threads until they are really dead.
	(inf_ttrace_wait): After stopping the whole process, delete any
	dying thread that is really dead by now.
	(inf_ttrace_thread_alive): Return 1.
	(inf_ttrace_extra_thread_info): New.
	(inf_ttrace_target): Register inf_ttrace_extra_thread_info.

---
 gdb/inf-ttrace.c |  118 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 30 deletions(-)

Index: src/gdb/inf-ttrace.c
===================================================================
--- src.orig/gdb/inf-ttrace.c	2008-08-09 15:10:27.000000000 +0100
+++ src/gdb/inf-ttrace.c	2008-08-09 23:24:52.000000000 +0100
@@ -33,6 +33,7 @@
 #include "gdb_string.h"
 #include <sys/mman.h>
 #include <sys/ttrace.h>
+#include <signal.h>
 
 #include "inf-child.h"
 #include "inf-ttrace.h"
@@ -801,52 +802,85 @@ inf_ttrace_kill (void)
   target_mourn_inferior ();
 }
 
+/* Check is a dying thread is dead by now, and delete it from GDBs
+   thread list if so.  */
 static int
-inf_ttrace_resume_callback (struct thread_info *info, void *arg)
+inf_ttrace_delete_dead_threads_callback (struct thread_info *info, void *arg)
 {
-  if (!ptid_equal (info->ptid, inferior_ptid) && !is_exited (info->ptid))
-    {
-      pid_t pid = ptid_get_pid (info->ptid);
-      lwpid_t lwpid = ptid_get_lwp (info->ptid);
+  lwpid_t lwpid;
+  struct inf_ttrace_private_thread_info *p;
 
-      if (ttrace (TT_LWP_CONTINUE, pid, lwpid, TT_NOPC, 0, 0) == -1)
-	perror_with_name (("ttrace"));
-    }
+  if (is_exited (info->ptid))
+    return 0;
+
+  lwpid = ptid_get_lwp (info->ptid);
+  p = (struct inf_ttrace_private_thread_info *) info->private;
+
+  /* Check if an lwp that was dying is still there or not.  */
+  if (p->dying && (kill (lwpid, 0) == -1))
+    /* It's gone now.  */
+    delete_thread (info->ptid);
 
   return 0;
 }
 
+/* Resume the lwp pointed to by INFO, with REQUEST, and pass it signal
+   SIG.  */
+
+static void
+inf_ttrace_resume_lwp (struct thread_info *info, ttreq_t request, int sig)
+{
+  pid_t pid = ptid_get_pid (info->ptid);
+  lwpid_t lwpid = ptid_get_lwp (info->ptid);
+
+  if (ttrace (request, pid, lwpid, TT_NOPC, sig, 0) == -1)
+    {
+      struct inf_ttrace_private_thread_info *p
+	= (struct inf_ttrace_private_thread_info *) info->private;
+      if (p->dying && errno == EPROTO)
+	/* This is expected, it means the dying lwp is really gone
+	   by now.  If ttrace had an event to inform the debugger
+	   the lwp is really gone, this wouldn't be needed.  */
+	delete_thread (info->ptid);
+      else
+	/* This was really unexpected.  */
+	perror_with_name (("ttrace"));
+    }
+}
+
+/* Callback for iterate_over_threads.  */
+
 static int
-inf_ttrace_delete_dying_threads_callback (struct thread_info *info, void *arg)
+inf_ttrace_resume_callback (struct thread_info *info, void *arg)
 {
-  if (((struct inf_ttrace_private_thread_info *)info->private)->dying == 1)
-    delete_thread (info->ptid);
+  if (!ptid_equal (info->ptid, inferior_ptid) && !is_exited (info->ptid))
+    inf_ttrace_resume_lwp (info, TT_LWP_CONTINUE, 0);
+
   return 0;
 }
 
 static void
 inf_ttrace_resume (ptid_t ptid, int step, enum target_signal signal)
 {
-  pid_t pid = ptid_get_pid (ptid);
-  lwpid_t lwpid = ptid_get_lwp (ptid);
+  int resume_all;
   ttreq_t request = step ? TT_LWP_SINGLE : TT_LWP_CONTINUE;
   int sig = target_signal_to_host (signal);
+  struct thread_info *info;
 
-  if (pid == -1)
-    {
-      pid = ptid_get_pid (inferior_ptid);
-      lwpid = ptid_get_lwp (inferior_ptid);
-    }
+  /* A specific PTID means `step only this process id'.  */
+  resume_all = (ptid_equal (ptid, minus_one_ptid));
 
-  if (ttrace (request, pid, lwpid, TT_NOPC, sig, 0) == -1)
-    perror_with_name (("ttrace"));
-
-  if (ptid_equal (ptid, minus_one_ptid))
-    {
-      /* Let all the other threads run too.  */
-      iterate_over_threads (inf_ttrace_resume_callback, NULL);
-      iterate_over_threads (inf_ttrace_delete_dying_threads_callback, NULL);
-    }
+  /* If resuming all threads, it's the current thread that should be
+     handled specially.  */
+  if (resume_all)
+    ptid = inferior_ptid;
+
+  info = thread_find_pid (ptid);
+  inf_ttrace_resume_lwp (info, request, sig);
+
+  if (resume_all)
+    /* Let all the other threads run too.  */
+    iterate_over_threads (inf_ttrace_resume_callback, NULL);
 }
 
 static ptid_t
@@ -1075,6 +1109,16 @@ inf_ttrace_wait (ptid_t ptid, struct tar
   if (ttrace (TT_PROC_STOP, tts.tts_pid, 0, 0, 0, 0) == -1)
     perror_with_name (("ttrace"));
 
+  /* Now that the whole process is stopped, check if any dying thread
+     is really dead by now.  If a dying thread is still alive, it will
+     be stopped too, and will still show up in `info threads', tagged
+     with "(Exiting)".  We could make `info threads' prune dead
+     threads instead via inf_ttrace_thread_alive, but doing this here
+     has the advantage that a frontend is notificed sooner of thread
+     exits.  Note that a dying lwp is still alive, it still has to be
+     resumed, like any other lwp.  */
+  iterate_over_threads (inf_ttrace_delete_dead_threads_callback, NULL);
+
   return ptid;
 }
 
@@ -1145,9 +1189,22 @@ inf_ttrace_files_info (struct target_ops
 static int
 inf_ttrace_thread_alive (ptid_t ptid)
 {
-  struct thread_info *ti;
-  ti = find_thread_pid (ptid);
-  return !(((struct inf_ttrace_private_thread_info *)ti->private)->dying);
+  return 1;
+}
+
+/* Return a string describing the state of the thread specified by
+   INFO.  */
+
+static char *
+inf_ttrace_extra_thread_info (struct thread_info *info)
+{
+  struct inf_ttrace_private_thread_info* private =
+    (struct inf_ttrace_private_thread_info *) info->private;
+
+  if (private != NULL && private->dying)
+    return "Exiting";
+
+  return NULL;
 }
 
 static char *
@@ -1188,6 +1245,7 @@ inf_ttrace_target (void)
   t->to_follow_fork = inf_ttrace_follow_fork;
   t->to_mourn_inferior = inf_ttrace_mourn_inferior;
   t->to_thread_alive = inf_ttrace_thread_alive;
+  t->to_extra_thread_info = inf_ttrace_extra_thread_info;
   t->to_pid_to_str = inf_ttrace_pid_to_str;
   t->to_xfer_partial = inf_ttrace_xfer_partial;
 

Re: ttrace: Protocal error

[Pedro Alves]

On Saturday 09 August 2008 23:45:23, Pedro Alves wrote:

> Hope I haven't broken anything badly.  I've never in my live logged in
> to an HP-UX system, so wear sunglasses.

And of course, I meant to say that this applies on top of my other
patch to register the main thread; and that this is an alternative
patch for consideration.  It's fine with me to go the other simpler
route, as per your recent patch.

-- 
Pedro Alves

Re: ttrace: Protocal error

[John David Anglin]

> On Saturday 09 August 2008 23:45:23, Pedro Alves wrote:
> 
> > Hope I haven't broken anything badly.  I've never in my live logged in
> > to an HP-UX system, so wear sunglasses.
> 
> And of course, I meant to say that this applies on top of my other
> patch to register the main thread; and that this is an alternative
> patch for consideration.  It's fine with me to go the other simpler
> route, as per your recent patch.

I always work at a distance in case thing blow...

My patch isn't a perfect solution.  While EPROTO is the most
frequent error when resuming, I have also seen EINVAL and ESRCH.

Then, if I pound real hard trying to make vla6.x3g fail:

(gdb) r
Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 
vfork: Resource temporarily unavailable.
(gdb) r
Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 
vfork: Resource temporarily unavailable.
...

-bash-3.2$ ps -ef|grep vla|wc
73 585 8121

For some reason, the vla6.x3g processes aren't exiting when run under gdb
in spite of the fact that gdb says they exited normally.

I'll look at your patches.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: ttrace: Protocal error

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

On Saturday 09 August 2008 15:51:16, Pedro Alves wrote:
> Also, when we detect a TTEVT_LWP_CREATE, TTEVT_LWP_EXIT or
> TTEVT_LWP_TERMINATE, The ttrace docs indicate that only one lwp is
> stopped.  There's no reason to stop all lwps, and return
> TARGET_WAITKING_SPURIOUS, only to resume all lwps again.
> This just adds overhead and messes more with the
> scheduling of the inferior than needed.  We could just resume
> the stopped lwp, and return TARGET_WAITKIND_IGNORE.

Here's a patch to do this.  It applies on top of the other I just sent,
which itself applied on top of my other patch to always register
the main thread.

-- 
Pedro Alves

[-- Attachment #2: fix_daves_bug_part_2.diff --]
[-- Type: text/x-diff, Size: 2720 bytes --]

2008-08-09  Pedro Alves  <pedro@codesourcery.com>

	* inf-ttrace.c (inf_ttrace_wait): On TTEVT_LWP_CREATE and
	LWP_TERMINATE, resume the caller thread.  On TTEVT_LWP_CREATE,
	TTEVT_LWP_EXIT and TTEVT_LWP_TERMINATE, don't stop the whole
	process, and return TARGET_WAITKIND_IGNORE.

---
 gdb/inf-ttrace.c |   26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

Index: src/gdb/inf-ttrace.c
===================================================================
--- src.orig/gdb/inf-ttrace.c	2008-08-09 23:15:40.000000000 +0100
+++ src/gdb/inf-ttrace.c	2008-08-09 23:15:53.000000000 +0100
@@ -1041,7 +1041,12 @@ inf_ttrace_wait (ptid_t ptid, struct tar
 	      sizeof (struct inf_ttrace_private_thread_info));
       inf_ttrace_num_lwps++;
       ptid = ptid_build (tts.tts_pid, tts.tts_lwpid, 0);
-      break;
+      /* Let the lwp_create-caller thread continue.  */
+      ttrace (TT_LWP_CONTINUE, ptid_get_pid (ptid),
+              ptid_get_lwp (ptid), TT_NOPC, 0, 0);
+      /* Return without stopping the whole process.  */
+      ourstatus->kind = TARGET_WAITKIND_IGNORE;
+      return ptid;
 
     case TTEVT_LWP_EXIT:
       if (print_thread_events)
@@ -1050,22 +1055,31 @@ inf_ttrace_wait (ptid_t ptid, struct tar
       gdb_assert (ti != NULL);
       ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
       inf_ttrace_num_lwps--;
+      /* Let the thread really exit.  */
       ttrace (TT_LWP_CONTINUE, ptid_get_pid (ptid),
               ptid_get_lwp (ptid), TT_NOPC, 0, 0);
-      /* If we don't return -1 here, core GDB will re-add the thread.  */
-      ptid = minus_one_ptid;
-      break;
+      /* Return without stopping the whole process.  */
+      ourstatus->kind = TARGET_WAITKIND_IGNORE;
+      return ptid;
 
     case TTEVT_LWP_TERMINATE:
       lwpid = tts.tts_u.tts_thread.tts_target_lwpid;
       ptid = ptid_build (tts.tts_pid, lwpid, 0);
-      printf_filtered(_("[%s has been terminated]\n"), target_pid_to_str (ptid));
+      if (print_thread_events)
+	printf_unfiltered(_("[%s has been terminated]\n")
+			  target_pid_to_str (ptid));
       ti = find_thread_pid (ptid);
       gdb_assert (ti != NULL);
       ((struct inf_ttrace_private_thread_info *)ti->private)->dying = 1;
       inf_ttrace_num_lwps--;
+
+      /* Resume the lwp_terminate-caller thread.  */
       ptid = ptid_build (tts.tts_pid, tts.tts_lwpid, 0);
-      break;
+      ttrace (TT_LWP_CONTINUE, ptid_get_pid (ptid),
+              ptid_get_lwp (ptid), TT_NOPC, 0, 0);
+      /* Return without stopping the whole process.  */
+      ourstatus->kind = TARGET_WAITKIND_IGNORE;
+      return ptid;
 
     case TTEVT_SIGNAL:
       ourstatus->kind = TARGET_WAITKIND_STOPPED;

Re: ttrace: Protocal error

[Joel Brobecker]

> 2008-08-08  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
> 
> 	* inf-ttrace.c (inf_ttrace_resume_callback): Don't resume dying thread.

That looks OK, but did you check the effect on the testsuite results?
I want to make sure that we're not causing a program to hang at the end
just because a dying thread wasn't allowed to die.

Also, I wonder why we're deleting the "dying" threads from the thread list
at resume time:

   static void
   inf_ttrace_resume (ptid_t ptid, int step, enum target_signal signal)
   {
     [...]
     if (ptid_equal (ptid, minus_one_ptid) && inf_ttrace_num_lwps > 0)
       {
         /* Let all the other threads run too.  */
         iterate_over_threads (inf_ttrace_resume_callback, NULL);
         iterate_over_threads (inf_ttrace_delete_dying_threads_callback, NULL);
       }
   }

-- 
Joel

Re: ttrace: Protocal error

[John David Anglin]

> (gdb) r
> Starting program: /mnt/gnu/gcc/objdir/hppa2.0w-hp-hpux11.11/libgomp/testsuite/vla6.x3g 

Trying some really old versions of gdb, I see this problem has been
around for a long time.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [4/7] Adjust the ttrace target (HP-UX) to always register the

[John David Anglin]

> Once these are fixed, the set seems to work (i.e., they
> fix the problem of ttrace errors when resuming threads).

I should have said that there is one remaining problem that I have
seen in testing these changes.  Processes run under gdb don't exit
until gdb exits.  This may or may not be related to the issues
being addressed by Pedro's changes.  This problem isn't new.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH] Fix dwarf register column to gdb register mapping

[John David Anglin]

> > When I enabled the dwarf debugging support in gdb before i caused
> > various errors and warnings about incorrect cfi. Do you still see that
> > with this enabled? Is it compiler dependent? Because of those warnings
> > and errors I had disabled the dwarf stuff on hppa before.
> 
> Investigating some more, I see there's a problem with the floating
> point registers.  A break on wack_double demonstrates the problem
> on both hppa64-hpux and hppa-linux:

The following change fixes the problem.  The confusion was between "dbx"
register numbers and "dwarf" register columns.  The hppa64 implementation
was essentially correct except for a botched if statement.  I tried to
make it clear in the change that the mapping is for dbx registers.

As far as I can tell, the dwarf debugging support is now enabled by default
and the ifdef'd out code no longer works to disable it.  So, the mapping
had to be fixed.

Committed as follows.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-09-14  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* hppa-linux-tdep.c (hppa_dwarf_reg_to_regnum): Remove surrounding
	"#if 0" "#endif".  Fix mapping of DWARF DBX registers to GDB registers.
	Correct arguments and improve comments.
	(hppa_linux_init_abi): Call set_gdbarch_dwarf2_reg_to_regnum.  Delete
	disabled code.
	* hppa-tdep.c (hppa64_dwarf_reg_to_regnum): Fix check for floating
	point DBX register, change error to warning, and improve comments.

Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.32
diff -u -3 -p -r1.32 hppa-linux-tdep.c
--- hppa-linux-tdep.c	14 Sep 2008 14:08:42 -0000	1.32
+++ hppa-linux-tdep.c	15 Sep 2008 01:33:28 -0000
@@ -34,24 +34,21 @@
 
 #include "elf/common.h"
 
-#if 0
-/* Convert DWARF register number REG to the appropriate register
-   number used by GDB.  */
+/* Map DWARF DBX register numbers to GDB register numbers.  */
 static int
-hppa_dwarf_reg_to_regnum (int reg)
+hppa_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
 {
-  /* registers 0 - 31 are the same in both sets */
-  if (reg < 32)
+  /* The general registers and the sar are the same in both sets.  */
+  if (reg <= 32)
     return reg;
 
-  /* dwarf regs 32 to 85 are fpregs 4 - 31 */
-  if (reg >= 32 && reg <= 85)
-    return HPPA_FP4_REGNUM + (reg - 32);
+  /* fr4-fr31 (left and right halves) are mapped from 72.  */
+  if (reg >= 72 && reg <= 72 + 28 * 2)
+    return HPPA_FP4_REGNUM + (reg - 72);
 
-  warning (_("Unmapped DWARF Register #%d encountered."), reg);
+  warning (_("Unmapped DWARF DBX Register #%d encountered."), reg);
   return -1;
 }
-#endif
 
 static void
 hppa_linux_target_write_pc (struct regcache *regcache, CORE_ADDR v)
@@ -545,12 +545,7 @@ hppa_linux_init_abi (struct gdbarch_info
   set_gdbarch_regset_from_core_section
     (gdbarch, hppa_linux_regset_from_core_section);
 
-#if 0
-  /* Dwarf-2 unwinding support.  Not yet working.  */
   set_gdbarch_dwarf2_reg_to_regnum (gdbarch, hppa_dwarf_reg_to_regnum);
-  frame_unwind_append_sniffer (gdbarch, dwarf2_frame_sniffer);
-  frame_base_append_sniffer (gdbarch, dwarf2_frame_base_sniffer);
-#endif
 
   /* Enable TLS support.  */
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.260
diff -u -3 -p -r1.260 hppa-tdep.c
--- hppa-tdep.c	11 Sep 2008 14:23:15 -0000	1.260
+++ hppa-tdep.c	15 Sep 2008 01:33:29 -0000
@@ -658,18 +658,19 @@ hppa64_register_name (struct gdbarch *gd
     return names[i];
 }
 
+/* Map dwarf DBX register numbers to GDB register numbers.  */
 static int
 hppa64_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
 {
-  /* r0-r31 and sar map one-to-one.  */
+  /* The general registers and the sar are the same in both sets.  */
   if (reg <= 32)
     return reg;
 
   /* fr4-fr31 are mapped from 72 in steps of 2.  */
-  if (reg >= 72 || reg < 72 + 28 * 2)
+  if (reg >= 72 && reg < 72 + 28 * 2 && !(reg & 1))
     return HPPA64_FP4_REGNUM + (reg - 72) / 2;
 
-  error ("Invalid DWARF register num %d.", reg);
+  warning (_("Unmapped DWARF DBX Register #%d encountered."), reg);
   return -1;
 }
 

Re: [PATCH] Fix hppa_linux_sigtramp_frame_unwind_cache

[John David Anglin]

> No, you are correct.  The long long field will be aligned at an 8-byte
> boundary.  I will keep the pad.

Committed the following after retesting.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-09-14  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* hppa-linux-tdep.c (hppa_linux_sigtramp_frame_unwind_cache): Record
	HPPA_IPSW_REGNUM and HPPA_SAR_REGNUM values.

Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.32
diff -u -3 -p -r1.32 hppa-linux-tdep.c
--- hppa-linux-tdep.c	14 Sep 2008 14:08:42 -0000	1.32
+++ hppa-linux-tdep.c	15 Sep 2008 00:50:52 -0000
@@ -227,7 +227,8 @@ hppa_linux_sigtramp_frame_unwind_cache (
   /* Skip sc_flags.  */
   scptr += 4;
 
-  /* GR[0] is the psw, we don't restore that.  */
+  /* GR[0] is the psw.  */
+  info->saved_regs[HPPA_IPSW_REGNUM].addr = scptr;
   scptr += 4;
 
   /* General registers.  */
@@ -237,7 +238,7 @@ hppa_linux_sigtramp_frame_unwind_cache (
       scptr += 4;
     }
 
-  /* Pad.  */
+  /* Pad to long long boundary.  */
   scptr += 4;
 
   /* FP regs; FP0-3 are not restored.  */
@@ -262,6 +263,8 @@ hppa_linux_sigtramp_frame_unwind_cache (
   info->saved_regs[HPPA_PCOQ_TAIL_REGNUM].addr = scptr;
   scptr += 4;
 
+  info->saved_regs[HPPA_SAR_REGNUM].addr = scptr;
+
   info->base = get_frame_register_unsigned (this_frame, HPPA_SP_REGNUM);
 
   return info;

Re: [RFA] patch to add 'maint profile-gdb' command

[Andrew Cagney]

>  i think the command should always be present.  looking at the patch, it 
>> appears to have started out that way.
> 
> 
> I'd disagree.  First, these functions are not very portable.  Our
> very own RH 6.2 box that is sourceware.cygnus.com does not have
> moncontrol() or monstart(), and -pg doesn't seem to be usable at
> all.  My RH 7.1 box at home works fine.  MacOS X's FreeBSD works
> fine.  Second, you definitely don't want to compile gdb with -pg
> by default - the compiler inserts a bunch of bookkeeping code and
> functions, and that'll be a real performance penalty.

while the command is always present it wouldn't do much by default.  the 
function that implements the command throws an error.

is that making more sense?

> are you sure no changes to maint.exp are needed? not needing them feels 
>> wrong.
> 
> 
> The "help maint" test in maint.exp globs it up.  You don't want to include
> a check for maint profile-gdb in here because you'd get a FAIL when gdb
> was not configured --enable-profiling.

or to turn it around, will an --enable-profiling gdb have additional 
failures?  i don't think that enabling profiling should affect the test 
results.

andrew

Re: [RFA] patch to add 'maint profile-gdb' command

[Andrew Cagney]

>> gdb is pure iso c, the params forward decl isn't needed.  you may want 
>> to look at http://sources.redhat.com/gdb/ari/ for a check list of things 
>> recently zapped from gdb.
> 
> 
> The forward decl in maint.c?  Tom was following the convention of the
> other functions in that file..

the PARAMS() bit - ISO C is assumed (the forward decl must be added when 
needed, beyond that is now optional)

andrew

[RFA] Version 2 of patch to add 'maint profile-gdb' command

[Jason Molenda]

Here is a minor rewrite of the profiling patch.  I removed the
PARAMS, fixed the NEWS entry, and added a check for moncontrol()
with -pg - if this fails to compile (as it fails on sources.redhat.com),
an error message is spit out and the configure aborts.  If a warning
is desirable, it's a minor change from calling AC_MSG_ERROR to
AC_MSG_WARN, but I found that to be too subtle in my testing (the
warning was lost among the configure output and people would be
confused why their --enable-profiling gdb isn't actually profiling.)
I amended the gdbint.texinfo documentation to note that the gmon.out
file is overwritten each time gdb is started.

The one notable change I did not incorporate was compiling with
-pg, linking without -pg, and using monstartup() to enable the
profiling.  I have two problems with this - one, I _suspect_ (but
I do not know for certain) that linking without -pg and just calling
monstartup() is going to be unportable.  The second problem is
that, off-hand, I don't know how a program should determine the
start and end addresses of its TEXT section portably (which are
required to use monstartup).

I agree that using monstartup() would be convenient, as developers
would not constantly overwrite their existing gmon.out file whenever
they start gdb, but I'd like to see this patch committed and someone
who understands the mechanisms of profiling across different systems
could tackle it in the future.  In my opinion, monstartup() as a nice
refinement which a motivated person could tackle in the future.  I'd
rather not complicate the basic feature with this at this point.
This patch was written nearly two years ago - it'd be good to get
it integrated.

NB - autoconf and autoheader must both be run after applying this patch.

Thanks for your help,

Jason
From eliz@is.elta.co.il Sun Sep 16 22:36:00 2001
From: Eli Zaretskii <eliz@is.elta.co.il>
To: jason-swarelist@molenda.com
Cc: gdb-patches@sources.redhat.com, tromey@cygnus.com
Subject: Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command
Date: Sun, 16 Sep 2001 22:36:00 -0000
Message-id: <200109170536.HAA21988@is.elta.co.il>
References: <20010910003022.A21681@shell17.ba.best.com> <3B9CE0C6.5060700@cygnus.com> <20010910115244.A25119@shell17.ba.best.com> <3B9FACDA.6070407@cygnus.com> <20010916174433.A3258@shell17.ba.best.com>
X-SW-Source: 2001-09/msg00204.html
Content-length: 1311

> Date: Sun, 16 Sep 2001 17:44:33 -0700
> From: Jason Molenda <jason-swarelist@molenda.com>
> 
> The one notable change I did not incorporate was compiling with
> -pg, linking without -pg, and using monstartup() to enable the
> profiling.  I have two problems with this - one, I _suspect_ (but
> I do not know for certain) that linking without -pg and just calling
> monstartup() is going to be unportable.  The second problem is
> that, off-hand, I don't know how a program should determine the
> start and end addresses of its TEXT section portably (which are
> required to use monstartup).

Emacs uses monstartup and manages to solve both of these problems.
See the file src/emacs.c (search for "etext") for the details.

> diff -u -p -r1.39 gdbint.texinfo
> --- doc/gdbint.texinfo	8 Sep 2001 10:53:45 -0000	1.39
> +++ doc/gdbint.texinfo	17 Sep 2001 00:34:26 -0000

I'd like the documentation of the "maint profile-gdb" command to be in
gdb.texinfo, not gdbint.texinfo.  The latter doesn't document user
commands, and users might not readily know to look in gdbint.  (It's
okay to have it described in both manuals, if you think it will be
useful.)

> +
> +@section Profiling GDB

Please replace "GDB" with "@value{GDBN}" everywhere.

Other than the last two comments, the patches to the docs are
approved.

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Michael Snyder]

Eli Zaretskii wrote:

> I'd like the documentation of the "maint profile-gdb" command to be in
> gdb.texinfo, not gdbint.texinfo.  The latter doesn't document user
> commands, and users might not readily know to look in gdbint.  (It's
> okay to have it described in both manuals, if you think it will be
> useful.)

At first I thought this was weird, but it's not.
Maintainer commands are documented in the user docs, not
the internals docs.  It has to do with maintaining gdb, 
not with gdb internals.  Good call.

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

> 
> Emacs uses monstartup and manages to solve both of these problems.
> See the file src/emacs.c (search for "etext") for the details.

from emacs 20.7:

>   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
>      FreeBSD.  It might work on some other systems too.
>      Give it a try and tell me if it works on your system.  */
> #ifdef __FreeBSD__
> #ifdef PROFILING
>   if (initialized)
>     {
>       extern void _mcleanup ();       
>       extern char etext;
>       extern Lisp_Object Fredraw_frame ();
>       atexit (_mcleanup);
>       /* This uses Fredraw_frame because that function
>          comes first in the Emacs executable.
>          It might be better to use something that gives
>          the start of the text segment, but start_of_text
>          is not defined on all systems now.  */
>       monstartup (Fredraw_frame, &etext);
>     }
>   else
>     moncontrol (0);
> #endif
> #endif

I get the feeling that this is work in progress.  I'd, for the moment, 
suggest using Jason's mechanism - it isn't perfect  but it works. 
Someone can later make it more fancy.

Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Tue, 18 Sep 2001 01:43:02 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> > Emacs uses monstartup and manages to solve both of these problems.
> > See the file src/emacs.c (search for "etext") for the details.
> 
> from emacs 20.7:
> 
> >   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
> >      FreeBSD.  It might work on some other systems too.
> >      Give it a try and tell me if it works on your system.  */
[...]
> I get the feeling that this is work in progress.

This file is almost 2 years old.  Since then, this code was proven to
run unmodified on quite a few platforms.

Anyway, I don't want to continue arguing about this.  As long as you
know that the problems Jason raised can be solved with simple and
quite portable code, all the rest is your judgement call.

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Jason Molenda]

Sorry for the delay following up, I was sidetracked by some other 
things for a few days.

On Tue, Sep 18, 2001 at 09:51:16AM +0300, Eli Zaretskii wrote:

> > from emacs 20.7:
> > 
> > >   /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
> > >      FreeBSD.  It might work on some other systems too.
> > >      Give it a try and tell me if it works on your system.  */
> [...]
> > I get the feeling that this is work in progress.
> 
> This file is almost 2 years old.  Since then, this code was proven to
> run unmodified on quite a few platforms.

I'm a little hesitant to use this emacs code... besides having an
#ifdef FreeBSD around it, it uses the address of a certain function
for the beginning of the text section -- hardcoded.  So unless
someone can guess which one of gdb's functions is going to end up
at the beginning of .text, I don't see how that'd work.

It looks like emacs (in 20.7) is guarding all of these calls to
monstartup() and moncontrol() with #ifdef PROFILING.

I don't follow emacs development at all, so it's possible the
development sources are significantly different.

> Anyway, I don't want to continue arguing about this.  

I appreciate the time you're taking with this.  Okay, I would have
been happier if someone had just said "Approved!" and it had gone
in immediately :-), but it'll be a better patch by the time it does
get committed.

> As long as you
> know that the problems Jason raised can be solved with simple and
> quite portable code, all the rest is your judgement call.

I'm willing to believe that there are portable ways to do everything
we're talking about, but _I_ don't know them, and the emacs 20.7 code
doesn't show any examples of this...

I'll be updating the profiling patch and sending it again soon.

Jason

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

>> > > /* Gerd Moellmann <gerd@acm.org> says this makes profiling work on
>> > > FreeBSD.  It might work on some other systems too.
>> > > Give it a try and tell me if it works on your system.  */

For what its worth.   I think this part of this discussion thread should 
be converted into a CR - something about profile always creating a 
profile file and notes on possible solutions.

Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Mon, 24 Sep 2001 17:41:38 -0400
> From: Andrew Cagney <ac131313@cygnus.com>
> 
> For what its worth.   I think this part of this discussion thread should 
> be converted into a CR

What's a CR?  You didn't mean CR the character, \r, did you? ;-)

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

> 
> What's a CR?  You didn't mean CR the character, \r, did you? 

Sorry, Change Reqest, one of the PR categories in the bug database. If 
I'm not careful, I'll be refering to them as SPRs (system performance 
reports) next ....

Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Mon, 24 Sep 2001 13:32:15 -0700
> From: Jason Molenda <jason@molenda.com>
> 
> I'm a little hesitant to use this emacs code... besides having an
> #ifdef FreeBSD around it, it uses the address of a certain function
> for the beginning of the text section -- hardcoded.  So unless
> someone can guess which one of gdb's functions is going to end up
> at the beginning of .text, I don't see how that'd work.

The first function in the binary is the first function the linker
sees.  If this is platform-dependent in GDB, we could simply arrange
for an empty function that every platform links in.

> Okay, I would have been happier if someone had just said "Approved!"
> and it had gone in immediately :-)

I can't be that someone: it's not my responsibility ;-)

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

> +  add_cmd ("profile-gdb", class_maintenance, maint_profile_gdb,
> +	   "Enable or disable profiling.",
> +	   &maintenancelist);

Jason, should have thought of this earlier, sorry.  Should this be 
``set/show maint profile-gdb''?  ``set/show remote ...'' provides 
cut/paste for implementing this.

As far as I can tell it isn't possible to examine the value of profile-gdb.

Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Fernando Nasser]

Andrew Cagney wrote:
> 
> > +  add_cmd ("profile-gdb", class_maintenance, maint_profile_gdb,
> > +        "Enable or disable profiling.",
> > +        &maintenancelist);
> 
> Jason, should have thought of this earlier, sorry.  Should this be
> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
> cut/paste for implementing this.
> 
> As far as I can tell it isn't possible to examine the value of profile-gdb.
> 

It seems to be a general problem with "maint" things that are settings
(instead of an action).

We should eventually create a "set/show maint" thing similar to
"set/show debug" (humm, what is the difference here?).
I would leave it for a separate patch which would do it for all
the maint related settings.


BTW, "maint profile-gdb" is redundant.  maint commands are supposed to be
introspective already.  The target is always gdb itself, never the target.
Someone mentioned that the "-gdb" should be dropped and I agree.  But I do
not mind if it stays as it is as "maint profile" will work anyways.


-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

>> Jason, should have thought of this earlier, sorry.  Should this be
>> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
>> cut/paste for implementing this.
>> 
>> As far as I can tell it isn't possible to examine the value of profile-gdb.
>> 
> 
> 
> It seems to be a general problem with "maint" things that are settings
> (instead of an action).
> 
> We should eventually create a "set/show maint" thing similar to
> "set/show debug" (humm, what is the difference here?).
> I would leave it for a separate patch which would do it for all
> the maint related settings.


If you're ok with ``set/show maint ...'' I can clone ``set/show remote 
...'' into what is needed.

Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Fernando Nasser]

Andrew Cagney wrote:
> 
> >> Jason, should have thought of this earlier, sorry.  Should this be
> >> ``set/show maint profile-gdb''?  ``set/show remote ...'' provides
> >> cut/paste for implementing this.
> >>
> >> As far as I can tell it isn't possible to examine the value of profile-gdb.
> >>
> >
> >
> > It seems to be a general problem with "maint" things that are settings
> > (instead of an action).
> >
> > We should eventually create a "set/show maint" thing similar to
> > "set/show debug" (humm, what is the difference here?).
> > I would leave it for a separate patch which would do it for all
> > the maint related settings.
> 
> If you're ok with ``set/show maint ...'' I can clone ``set/show remote
> ...'' into what is needed.
> 

I think it is a good idea. But we should leave the things that are
currently set with the maint command marked as deprecated and
with the appropriate message telling to use "set maint" instead.

Also, I would like someone else to support us on this move: Tom? Jason? Eli?

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Wed, 19 Sep 2001 10:07:55 -0400
> From: Fernando Nasser <fnasser@cygnus.com>
> 
> Also, I would like someone else to support us on this move: Tom? Jason? Eli?

Sorry, I probably wasn't following this thread closely enough: what
move did you have in mind?

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Fernando Nasser]

Eli Zaretskii wrote:
> 
> > Date: Wed, 19 Sep 2001 10:07:55 -0400
> > From: Fernando Nasser <fnasser@cygnus.com>
> >
> > Also, I would like someone else to support us on this move: Tom? Jason? Eli?
> 
> Sorry, I probably wasn't following this thread closely enough: what
> move did you have in mind?

We have some maint commands that actually set values.  Either turn things
on/off or set control variables (for instance, name of a log file).
As these are just plain commands, there is no way to examine the current
settings.

Andrew has proposed creating a class of set/show commands "set/show maint"
to replace those.  It would be used for instance to turn gdb self profiling
on/off.  I proposed that we use the deprecate the existent maint commands that
currently set things directly and add the new "set/show maint" for them.

So, for maintenance we would have:

maint <action> [<args>]
set maint <control var> <value>
show maint [<control var>]

All these are introspective commands that will examine/affect gdb in order
to debug, test or even to understand it.

Anyway, it is a modification on the current "maintenance" interface, that is
why I asked for some support beyond the two of us.

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Eli Zaretskii]

> Date: Wed, 19 Sep 2001 12:26:45 -0400
> From: Fernando Nasser <fnasser@cygnus.com>
> 
> So, for maintenance we would have:
> 
> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]

Ah, this seems a good idea.  Thanks for the explanations.

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Andrew Cagney]

> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]

just an aside to this, there is also

	set/show debug ....

but I don't think we need to change that to ``set/show maint debug 
...''.  ``set/show debug ...'' is very specific enable/disableing 
internal debug tracing.

enjoy,
Andrew

Re: [RFA] Version 2 of patch to add 'maint profile-gdb' command

[Kevin Buettner]

On Sep 19, 12:26pm, Fernando Nasser wrote:

> Andrew has proposed creating a class of set/show commands "set/show maint"
> to replace those.  It would be used for instance to turn gdb self profiling
> on/off.  I proposed that we use the deprecate the existent maint commands that
> currently set things directly and add the new "set/show maint" for them.
> 
> So, for maintenance we would have:
> 
> maint <action> [<args>]
> set maint <control var> <value>
> show maint [<control var>]
> 
> All these are introspective commands that will examine/affect gdb in order
> to debug, test or even to understand it.
> 
> Anyway, it is a modification on the current "maintenance" interface, that is
> why I asked for some support beyond the two of us.

I like having the maintenance commands all lumped under "maint".  That
way I can use tab completion to see all of the maintenance related commands
when I do

    (gdb) maint <TAB>

I think a set/show would be useful for some activities; couldn't
we add "maint set ..." and "maint show ..."  ?

Kevin

[PATCH] Fix hppa_linux_sigtramp_frame_unwind_cache

[John David Anglin]

The main fix in this patch is to remove the pad increment of scptr.  As
far as I know, there is no pad field in the sigcontext struct between the
general registers and the floating point registers.

I also recorded the PSW and SAR registers.  It might be a mistake to record
the PSW value, but I believe the SAR register definitely needs to be recorded.

Somewhat tested on hppa-linux but the gdb testsuite crashes 2.6.19.22.

Ok?

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

2008-09-13  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

	* hppa-linux-tdep.c (hppa_linux_sigtramp_frame_unwind_cache): Record
	HPPA_IPSW_REGNUM and HPPA_SAR_REGNUM values.  Remove pad increment.

Index: hppa-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
retrieving revision 1.31
diff -u -3 -p -r1.31 hppa-linux-tdep.c
--- hppa-linux-tdep.c	21 Aug 2008 13:19:18 -0000	1.31
+++ hppa-linux-tdep.c	13 Sep 2008 22:51:35 -0000
@@ -227,7 +229,8 @@ hppa_linux_sigtramp_frame_unwind_cache (
   /* Skip sc_flags.  */
   scptr += 4;
 
-  /* GR[0] is the psw, we don't restore that.  */
+  /* GR[0] is the psw.  */
+  info->saved_regs[HPPA_IPSW_REGNUM].addr = scptr;
   scptr += 4;
 
   /* General registers.  */
@@ -237,9 +240,6 @@ hppa_linux_sigtramp_frame_unwind_cache (
       scptr += 4;
     }
 
-  /* Pad.  */
-  scptr += 4;
-
   /* FP regs; FP0-3 are not restored.  */
   scptr += (8 * 4);
 
@@ -262,6 +262,9 @@ hppa_linux_sigtramp_frame_unwind_cache (
   info->saved_regs[HPPA_PCOQ_TAIL_REGNUM].addr = scptr;
   scptr += 4;
 
+  info->saved_regs[HPPA_SAR_REGNUM].addr = scptr;
+  scptr += 4;
+
   info->base = get_frame_register_unsigned (this_frame, HPPA_SP_REGNUM);
 
   return info;

Re: [PATCH] Fix hppa_linux_sigtramp_frame_unwind_cache

[Randolph Chung]

Hrm, this worries me.

The sigcontext structure looks like this:

struct sigcontext {
    unsigned long sc_flags;
    unsigned long sc_gr[32];
    unsigned long long sc_fr[32];
    unsigned long sc_iasq[2];
    unsigned long sc_iaoq[2];
    unsigned long sc_sar;
};

I am positive that when I wrote this the structure got padded between
sc_gr and sc_fr to keep the unsigned long long at a 8-byte boundary.
If this has changed now that means we've introduced a silent ABI
change....

The SAR stuff looks ok. I'm not very sure about restoring the IPSW either.

randolph


On Sat, Sep 13, 2008 at 4:57 PM, John David Anglin
<dave@hiauly1.hia.nrc.ca> wrote:
> The main fix in this patch is to remove the pad increment of scptr.  As
> far as I know, there is no pad field in the sigcontext struct between the
> general registers and the floating point registers.
>
> I also recorded the PSW and SAR registers.  It might be a mistake to record
> the PSW value, but I believe the SAR register definitely needs to be recorded.
>
> Somewhat tested on hppa-linux but the gdb testsuite crashes 2.6.19.22.
>
> Ok?
>
> Dave
> --
> J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
> National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)
>
> 2008-09-13  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
>
>        * hppa-linux-tdep.c (hppa_linux_sigtramp_frame_unwind_cache): Record
>        HPPA_IPSW_REGNUM and HPPA_SAR_REGNUM values.  Remove pad increment.
>
> Index: hppa-linux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-linux-tdep.c,v
> retrieving revision 1.31
> diff -u -3 -p -r1.31 hppa-linux-tdep.c
> --- hppa-linux-tdep.c   21 Aug 2008 13:19:18 -0000      1.31
> +++ hppa-linux-tdep.c   13 Sep 2008 22:51:35 -0000
> @@ -227,7 +229,8 @@ hppa_linux_sigtramp_frame_unwind_cache (
>   /* Skip sc_flags.  */
>   scptr += 4;
>
> -  /* GR[0] is the psw, we don't restore that.  */
> +  /* GR[0] is the psw.  */
> +  info->saved_regs[HPPA_IPSW_REGNUM].addr = scptr;
>   scptr += 4;
>
>   /* General registers.  */
> @@ -237,9 +240,6 @@ hppa_linux_sigtramp_frame_unwind_cache (
>       scptr += 4;
>     }
>
> -  /* Pad.  */
> -  scptr += 4;
> -
>   /* FP regs; FP0-3 are not restored.  */
>   scptr += (8 * 4);
>
> @@ -262,6 +262,9 @@ hppa_linux_sigtramp_frame_unwind_cache (
>   info->saved_regs[HPPA_PCOQ_TAIL_REGNUM].addr = scptr;
>   scptr += 4;
>
> +  info->saved_regs[HPPA_SAR_REGNUM].addr = scptr;
> +  scptr += 4;
> +
>   info->base = get_frame_register_unsigned (this_frame, HPPA_SP_REGNUM);
>
>   return info;
>

Re: [PATCH] Fix hppa_linux_sigtramp_frame_unwind_cache

[John David Anglin]

> The sigcontext structure looks like this:
> 
> struct sigcontext {
>     unsigned long sc_flags;
>     unsigned long sc_gr[32];
>     unsigned long long sc_fr[32];
>     unsigned long sc_iasq[2];
>     unsigned long sc_iaoq[2];
>     unsigned long sc_sar;
> };
> 
> I am positive that when I wrote this the structure got padded between
> sc_gr and sc_fr to keep the unsigned long long at a 8-byte boundary.

No, you are correct.  The long long field will be aligned at an 8-byte
boundary.  I will keep the pad.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)