Re: [RFC] Notes on QUIT and STREQ et.al.

[jtc@redback.com (J.T. Conklin)]

>>>>> "Kevin" == Kevin Buettner <kevinb@cygnus.com> writes:
Kevin> I haven't looked to see how often (or where) STREQ and STRCMP are
Kevin> used, but these macros compare the first characters inline in an
Kevin> attempt to improve performance.  Have you assessed the benefits of
Kevin> doing this?  (If these optimizations significantly improve
Kevin> performance, I think they should stay.)

I've found attempts to increase performance don't always accomplish
what they set out to do.  Testing the first characters inline avoids
function call overhead at the expense of increased code size.  If it
ends up thrashing the icache, overall performance could actually
signifcantly decrease.

Kevin> It seems to me that a decent STREQ macro should also test to see
Kevin> if the pointers are equal.  I.e,
Kevin>
Kevin>    #define STREQ(a,b) ((a == b) || (*(a) == *(b) ? !strcmp ((a), (b)) : 0))

Only if you expect that comparing identical strings has a high likelyhood.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From jtc@redback.com Mon Mar 13 12:06:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Andrew Cagney <ac131313@cygnus.com>
Cc: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [RFC] Notes on QUIT and STREQ et.al.
Date: Mon, 13 Mar 2000 12:06:00 -0000
Message-id: <5m1z5emy7b.fsf@jtc.redbacknetworks.com>
References: <38CCC819.1071F28E@cygnus.com>
X-SW-Source: 2000-03/msg00237.html
Content-length: 814

>>>>> "Andrew" == Andrew Cagney <ac131313@cygnus.com> writes:
Andrew> Look OK to everyone?

Fine with me.

In fact, I've wondered to myself whether the performance improvement
(or reduction, it's not clear that these types of optimizations will
pay off) justified the existance of the STR*() macros.  I suspect that
some of the assumptions have been invalidated since they were written.
Function call overhead isn't as bad on modern processors as it was on
the VAX and 68020, compilers can recognize str*() and emit their own
inline versions, etc.

Even if the testing the first character does have a modest performance
improvement, I'd rather that whenever a performance issue is found that
we spend the effort on algorithmic optimizations than micro-optimizing.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From jtc@redback.com Mon Mar 13 12:13:00 2000
From: jtc@redback.com (J.T. Conklin)
To: Daniel Berlin <dan@cgsoftware.com>
Cc: Kevin Buettner <kevinb@cygnus.com>, Andrew Cagney <ac131313@cygnus.com>, GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [RFC] Notes on QUIT and STREQ et.al.
Date: Mon, 13 Mar 2000 12:13:00 -0000
Message-id: <5mwvn6ljbd.fsf@jtc.redbacknetworks.com>
References: <Pine.LNX.4.10.10003130829420.6968-100000@localhost.localdomain>
X-SW-Source: 2000-03/msg00238.html
Content-length: 640

>>>>> "Daniel" == Daniel Berlin <dan@cgsoftware.com> writes:

>> 
>> I haven't looked to see how often (or where) STREQ and STRCMP are
>> used, but these macros compare the first characters inline in an
>> attempt to improve performance.  Have you assessed the benefits of
>> doing this?  (If these optimizations significantly improve
>> performance, I think they should stay.)

Daniel> I'll check out the performance.  I know GCC has a strlen
Daniel> expander, but no strcmp expander, so it might be a benefit.

I thought that GCC can expand strcmp() if the target has a cmpstrsi
pattern.

        --jtc

-- 
J.T. Conklin
RedBack Networks
From ac131313@cygnus.com Mon Mar 13 15:55:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Jim Kingdon <kingdon@redhat.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [RFC] Notes on QUIT and STREQ et.al.
Date: Mon, 13 Mar 2000 15:55:00 -0000
Message-id: <38CD7F9E.206D9351@cygnus.com>
References: <200003131412.PAA16094@landau.wins.uva.nl> <b4saaq07p.fsf@rtl.cygnus.com>
X-SW-Source: 2000-03/msg00239.html
Content-length: 938

Jim Kingdon wrote:

> > I'm not sure if we want STREQ to go.  I think that `STREQ (a, b)' is
> > both easier to read and easier to type than `strcmp (a, b) == 0'.
> 
> Well, perhaps it is because I have gotten used to the strcmp == 0
> idiom, but I find it to be pretty annoying to have to look up a macro
> like this (sure, it _probably_ is defined in the obvious way, but you
> don't know that for sure when digging into a new program).  Granted
> strcmp == 0 is hard to understand until/unless you know the standard C
> library well enough for it to be second nature.

Well I personally prefer the forms:

	strcmp() == 0 (read: strcmp () equal)
and	strcmp() != 0 (read: strcmp () not-equal)

over ``strcmp()'' and ``!strcmp()'' as they offer a queue to the
programer but even then that style isn't a requirement.

Like you, the one I don't trust is STREQ().  I'm never 100% certain what
that macro is doing behind my back :-)

	Andrew
From ac131313@cygnus.com Mon Mar 13 15:59:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: jtc@redback.com
Cc: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [RFC] Notes on QUIT and STREQ et.al.
Date: Mon, 13 Mar 2000 15:59:00 -0000
Message-id: <38CD809E.86938653@cygnus.com>
References: <38CCC819.1071F28E@cygnus.com> <5m1z5emy7b.fsf@jtc.redbacknetworks.com>
X-SW-Source: 2000-03/msg00240.html
Content-length: 285

"J.T. Conklin" wrote:

> Even if the testing the first character does have a modest performance
> improvement, I'd rather that whenever a performance issue is found that
> we spend the effort on algorithmic optimizations than micro-optimizing.

Mind if I lift this paragraph?

	Andrew
From ac131313@cygnus.com Mon Mar 13 16:51:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: "H . J . Lu" <hjl@valinux.com>
Cc: binutils@sourceware.cygnus.com, egcs-patches@egcs.cygnus.com, GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: A patch for toplevel Makefile.in
Date: Mon, 13 Mar 2000 16:51:00 -0000
Message-id: <38CD8CF3.CA81E01@cygnus.com>
References: <20000310125542.A6624@valinux.com>
X-SW-Source: 2000-03/msg00241.html
Content-length: 446

"H . J . Lu" wrote:
> 
> Hi,
> 
> I checked in the patch enclosed here into binutils. The detailed
> discussions can be found at
> 
> http://sourceware.cygnus.com/ml/binutils/2000-03/msg00116.html
> http://sourceware.cygnus.com/ml/binutils/2000-03/msg00133.html
> 
> and their followups. I suggest gcc should also consider this patch.

(To flog a dead horse...)
Please cross post this sort of change to
gdb-patches@sourceware.cygnus.com

	Andrew
From shebs@apple.com Mon Mar 13 18:17:00 2000
From: Stan Shebs <shebs@apple.com>
To: Andrew Cagney <ac131313@cygnus.com>
Cc: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [PATCH] Fix C++ overloading, add support for seeing through references.
Date: Mon, 13 Mar 2000 18:17:00 -0000
Message-id: <38CDA138.AFEA050E@apple.com>
References: <38CCBD3A.35459C1C@cygnus.com>
X-SW-Source: 2000-03/msg00242.html
Content-length: 529

Andrew Cagney wrote:
> 
> FYI,
> 
> I've committed the attatched from Daniel Berlin (except for minor
> edits).  It has a very interesting effect on the arm-elf -> arm-sim
> testsuite results:

Yup, that's why I was excited to have these changes in!  (I know, only the
geekiest of geeks would be excited about testsuite passes...)  I'd like
to vote a big thanks to Dan Berlin for doing this work - it will make
a real difference to our C++ users.

(Now we just need someone to tweak the expected testsuite results... :-) )

Stan
From dan@cgsoftware.com Mon Mar 13 18:25:00 2000
From: Daniel Berlin <dan@cgsoftware.com>
To: Fernando Nasser <fnasser@cygnus.com>
Cc: Daniel Berlin <dan@cgsoftware.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH]: add set/show debug, move gdb debugging flags into it
Date: Mon, 13 Mar 2000 18:25:00 -0000
Message-id: <1z5emgnu.fsf@dan.resnet.rochester.edu>
References: <Pine.LNX.4.10.10003130852170.6968-200000@localhost.localdomain> <38CD37F8.D1B20C40@cygnus.com>
X-SW-Source: 2000-03/msg00243.html
Content-length: 4597

Fernando Nasser <fnasser@cygnus.com> writes:

> Daniel,
> 
> Your patch is something we've been looking forward to.  In the perspective of the CLI (which I am the maintainer) I am
> eager to see it checked in.  It touches other files though, so I would rather wait for Andrew to see them overnight (his
> day).
> 
Sounds fine by me.

> However...  your patch is removing the old command formats.  I have not yet checked in David Whedon patch for
> deprecating commands (I've been *real* busy these days) so we can't
>use it yet.

Okay.
> 
> This leaves us with two choices: we wait a week or so and use David deprecating thing, or just leave the old commands
> alive for now.  In either case you would have to adjust your patch,
> I hope you don't mind.
No problem for me either way. I'm happy to adjust it.

I think it would make more sense to deprecate rather than remove, IMHO.
> 
> Thank you very much for doing this.  This "set xyxsdert$#@%$debug"
> commands were really weird.

Yeah.
I really like it much better this way.
--Dan
> 
> Best regards,
> Fernando
> 
> Daniel Berlin wrote:
> > 
> > Attached is a patch to add "set debug" and "show debug", and move the gdb
> > debugging stuff (targetdebug,expressiondebug,etc) into those lists.
> > 
> > I renamed targetdebug,expressiondebug,etc (all the debug settings i
> > moved), to remove the "debug" from their name, so you do "set debug target
> > 1" rather than "set debug targetdebug 1".
> > 
> > The new command lists are named setdebuglist/showdebuglist, and are in
> > command.c.
> > I put set_debug and show_debug into command.c, for lack of a better place.
> > The point of all this is to make it much easier to see what debugging
> > flags you can set for GDB, and what they were set to. It also declutters
> > the set list.
> > I also enabled monitordebug, since it said int he comment it was waiting
> > for "set debug".
> > 
> > I have no idea who needs to approve this, since it touches a bunch of
> > stuff, but is only related to one domain so to speak.
> > 
> > I'm working on a changelog entry,  i wanted to get comments first.
> > 
> > Example of what you get with the patch installed:
> > (gdb) help set debug
> > Generic command for setting gdb debugging flags
> > List of set debug subcommands:
> > set debug arch -- Set architecture debugging
> > set debug event -- Set event debugging
> > set debug expression -- Set expression debugging
> > set debug remote -- Set debugging of remote protocol
> > set debug serial -- Set serial debugging
> > set debug target -- Set target debugging
> > set debug varobj -- Set varobj debugging
> > Type "help set debug" followed by set debug subcommand name for full
> > documentation.
> > Command name abbreviations are allowed if unambiguous.
> > (gdb)
> > 
> > (gdb) help show debug
> > Generic command for showing gdb debugging flags
> > List of show debug subcommands:
> > show debug arch -- Show architecture debugging
> > show debug event -- Show event debugging
> > show debug expression -- Show expression debugging
> > show debug remote -- Show debugging of remote protocol
> > show debug serial -- Show serial debugging
> > show debug target -- Show target debugging
> > show debug varobj -- Show varobj debugging
> > Type "help show debug" followed by show debug subcommand name for full
> > documenta
> > tion.
> > Command name abbreviations are allowed if unambiguous.
> > (gdb)
> > (gdb) show debug
> > arch:  Architecture debugging is 0.
> > event:  Event debugging is 0.
> > expression:  Expression debugging is 0.
> > remote:  Debugging of remote protocol is 0.
> > serial:  Serial debugging is 0.
> > target:  Target debugging is 0.
> > varobj:  Varobj debugging is 0.
> > (gdb) set debug
> > "set debug" must be followed by the name of a print subcommand.
> > List of set debug subcommands:
> > set debug arch -- Set architecture debugging
> > set debug event -- Set event debugging
> > set debug expression -- Set expression debugging
> > set debug remote -- Set debugging of remote protocol
> > set debug serial -- Set serial debugging
> > set debug target -- Set target debugging
> > set debug varobj -- Set varobj debugging
> > Type "help set debug" followed by set debug subcommand name for full
> > documentati
> > on.
> > Command name abbreviations are allowed if unambiguous.
> > (gdb)
> > 
> >   ------------------------------------------------------------------------------------------------------------------------
> >                     Name: setdebug.diff
> >    setdebug.diff    Type: Plain Text (TEXT/PLAIN)
> >                 Encoding: BASE64
From ac131313@cygnus.com Mon Mar 13 18:36:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: Re: [RFC] Notes on QUIT and STREQ et.al.
Date: Mon, 13 Mar 2000 18:36:00 -0000
Message-id: <38CDA55E.6C6C3918@cygnus.com>
References: <38CCC819.1071F28E@cygnus.com>
X-SW-Source: 2000-03/msg00244.html
Content-length: 2882

Andrew Cagney wrote:
> 
> The attatched spells out the long term prospects of both STREQ et.al.
> and QUIT.

FYI, I checked in the attatched.

I resisted the temptation to rewrite the macros eliminating ``?:'' :-)
Knowing my luck, I'd firstly get it wrong and  secondly it would turn
out that some code somewhere relied on the existing behavour.

	enjoy,
		Andrew
Mon Mar 13 21:21:41 2000  Andrew Cagney  <cagney@b1.cygnus.com>

	* defs.h (STREQ, STRCMP, STREQN): Document that these macros are
 	somewhat redundant.
	(QUIT): Note that this can probably be replaced by a function.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.11
diff -p -r1.11 defs.h
*** defs.h	2000/03/13 07:30:00	1.11
--- defs.h	2000/03/14 02:27:37
*************** extern int core_addr_greaterthan (CORE_A
*** 117,125 ****
  #define max(a, b) ((a) > (b) ? (a) : (b))
  #endif
  
! /* Gdb does *lots* of string compares.  Use macros to speed them up by
!    avoiding function calls if the first characters are not the same. */
  
  #define STRCMP(a,b) (*(a) == *(b) ? strcmp ((a), (b)) : (int)*(a) - (int)*(b))
  #define STREQ(a,b) (*(a) == *(b) ? !strcmp ((a), (b)) : 0)
  #define STREQN(a,b,c) (*(a) == *(b) ? !strncmp ((a), (b), (c)) : 0)
--- 117,140 ----
  #define max(a, b) ((a) > (b) ? (a) : (b))
  #endif
  
! /* Macros to do string compares.
  
+    NOTE: cagney/2000-03-14:
+ 
+    While old code can continue to refer to these macros, new code is
+    probably better off using strcmp() directly vis: ``strcmp() == 0''
+    and ``strcmp() != 0''.
+ 
+    This is because modern compilers can directly inline strcmp()
+    making the original justification for these macros - avoid function
+    call overhead by pre-testing the first characters
+    (``*X==*Y?...:0'') - redundant.
+ 
+    ``Even if [...] testing the first character does have a modest
+    performance improvement, I'd rather that whenever a performance
+    issue is found that we spend the effort on algorithmic
+    optimizations than micro-optimizing.'' J.T. */
+ 
  #define STRCMP(a,b) (*(a) == *(b) ? strcmp ((a), (b)) : (int)*(a) - (int)*(b))
  #define STREQ(a,b) (*(a) == *(b) ? !strcmp ((a), (b)) : 0)
  #define STREQN(a,b,c) (*(a) == *(b) ? !strncmp ((a), (b), (c)) : 0)
*************** extern int immediate_quit;
*** 152,157 ****
--- 167,179 ----
  extern int sevenbit_strings;
  
  extern void quit (void);
+ 
+ /* FIXME: cagney/2000-03-13: It has been suggested that the peformance
+    benefits of having a ``QUIT'' macro rather than a function are
+    marginal.  If the overhead of a QUIT function call is proving
+    significant then its calling frequency should probably be reduced
+    [kingdon].  A profile analyzing the current situtation is
+    needed. */
  
  #ifdef QUIT
  /* do twice to force compiler warning */
From ac131313@cygnus.com Mon Mar 13 20:35:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: James Ingham <jingham@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: RFA: Patch to add "auto-evaluate" variables to varobj package.
Date: Mon, 13 Mar 2000 20:35:00 -0000
Message-id: <38CDC165.1B53F521@cygnus.com>
References: <mf3dpygz5v.fsf@leda.cygnus.com>
X-SW-Source: 2000-03/msg00245.html
Content-length: 835

James Ingham wrote:
> What do you think?

Don't forget that there isn't any need to use PARAMS and K&R in new code
:-)

> *************** varobj_list (struct varobj ***varlist)
> *** 845,850 ****
> --- 863,872 ----
>      expression to see if it's changed.  Then go all the way
>      through its children, reconstructing them and noting if they've
>      changed.
> +    Return value:
> +     -1 if there was an error updating the varobj
> +     -2 if the type changed
> +     Otherwise it is the number of children + parent changed
> 
>      Only root variables can be updated... */

This is beginning to sound like something that desperately needs an
interface cleanup.
Perhaps the nr of children as an argument and an enum for the return
value.

And wow!  Going through the MI tests must have been a real ordeal!

	enjoy,
		Andrew
From ac131313@cygnus.com Mon Mar 13 21:01:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: GDB Patches <gdb-patches@sourceware.cygnus.com>
Subject: [PATCH] Add change-log variables to more MI files
Date: Mon, 13 Mar 2000 21:01:00 -0000
Message-id: <38CDC788.D9C8FE12@cygnus.com>
X-SW-Source: 2000-03/msg00246.html
Content-length: 1269

Just FYI,

I've checked in the attatched.  It should help keep ChangeLog entries
local.

	Andrew
Tue Mar 14 15:54:57 2000  Andrew Cagney  <cagney@b1.cygnus.com>

	* basics.c: Add EMACS local variable pointing change-log at this
 	file.
	* Makefile.in: Ditto

Index: testsuite/gdb.mi/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/Makefile.in,v
retrieving revision 1.1
diff -p -r1.1 Makefile.in
*** Makefile.in	2000/02/23 00:25:43	1.1
--- Makefile.in	2000/03/14 04:58:58
*************** distclean maintainer-clean realclean: cl
*** 18,20 ****
--- 18,24 ----
  
  Makefile: $(srcdir)/Makefile.in $(srcdir)/configure.in
  	$(SHELL) ./config.status --recheck
+ 
+ # Local variables: 
+ # change-log-default-name: "ChangeLog-mi"
+ # End: 
Index: testsuite/gdb.mi/basics.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/basics.c,v
retrieving revision 1.2
diff -p -r1.2 basics.c
*** basics.c	2000/03/06 21:33:38	1.2
--- basics.c	2000/03/14 04:58:58
*************** main ()
*** 37,41 ****
    return 0;
  }
  
! 
  
--- 37,45 ----
    return 0;
  }
  
! /*
! Local variables: 
! change-log-default-name: "ChangeLog-mi"
! End: 
! */
  
From ac131313@cygnus.com Mon Mar 13 23:15:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: Eli Zaretskii <eliz@is.elta.co.il>, gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH] GDB command-line switches and annotations docs
Date: Mon, 13 Mar 2000 23:15:00 -0000
Message-id: <38CDE6E6.3AF7FAE5@cygnus.com>
References: <200003101347.IAA21898@indy.delorie.com> <14537.5048.99180.910863@kwikemart.cygnus.com>
X-SW-Source: 2000-03/msg00247.html
Content-length: 597

Elena Zannoni wrote:
> 
> Eli Zaretskii writes:
>  >
>  > Here are patches to gdb.texinfo and annotate.texi which add indexing
>  > to command-line switches, document some switches that were not in the
>  > manual, and make annotate.texi part of GDB manual.
>  >
>  > (Are there plans to make gdbmi.texi be part of the manual as well?)
>  >
> 
> Eventually yes (Andrew?), right now it is not in prime time form yet. It is
> still very rough work in progress.

Yes its rough but still certainly better than nothing.
I guess the need to the MI commands against the doco becomes a FIXME :-(

	Andrew
From ac131313@cygnus.com Mon Mar 13 23:41:00 2000
From: Andrew Cagney <ac131313@cygnus.com>
To: Elena Zannoni <ezannoni@cygnus.com>
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: PATCH: printing elements of typedef'ed arrays
Date: Mon, 13 Mar 2000 23:41:00 -0000
Message-id: <38CDED0E.2FBE16B6@cygnus.com>
References: <14535.62150.908914.943283@kwikemart.cygnus.com>
X-SW-Source: 2000-03/msg00248.html
Content-length: 1079

Elena Zannoni wrote:
> 
> [Sorry, wrong list...]
> 
> When an array is typedeffed, like in this example:
> 
> typedef long ArrayLong [10];
> ArrayLong a1;
> 
> typedef struct s
> {
>  int a;
>  int b;
> } structure;
> 
> long a2 [10];
> structure s1;
> 
> int main (void)
> {
>     return 0;
> }
> 
> Gdb cannot print individual elements of the array a1:
> 
> (gdb) p a1
> $1 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
> (gdb) p a1[0]
> $2 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}  <<<< is incorrect
> (gdb) p a2
> $3 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
> (gdb) p a2[0]
> $4 = 0
> (gdb)
> 
> The following patch takes care of this.
> I tested it on solaris and showed no regressions.
> OK to check in?
> 
> Elena

Just two cross checks:

	o	the existing testsuite
		checks that the printing of
		character strings doesn't change
		and the patch didn't change the
		behavour

		(I've a foggy memory about a great
		debate where the printing of strings
		was made special in some way)

	o	you've sneekered in a testsuite
		change (to printcmd.exp?) :-)

Andrew

PS: I guess I'm the defacto maintainer.
From eliz@delorie.com Tue Mar 14 01:32:00 2000
From: Eli Zaretskii <eliz@delorie.com>
To: ac131313@cygnus.com
Cc: gdb-patches@sourceware.cygnus.com
Subject: Re: [PATCH] Add change-log variables to more MI files
Date: Tue, 14 Mar 2000 01:32:00 -0000
Message-id: <200003140932.EAA28159@indy.delorie.com>
References: <38CDC788.D9C8FE12@cygnus.com>
X-SW-Source: 2000-03/msg00249.html
Content-length: 373

> I've checked in the attatched.  It should help keep ChangeLog entries
> local.
[snip]
> + 
> + # Local variables: 
> + # change-log-default-name: "ChangeLog-mi"
> + # End: 

You _are_ aware that ChangeLog and ChangeLog-mi both map to the same
name "changelo" when truncated to 8+3 limits, right?

Is it conceivable to rename ChangeLog-mi to ChangeLog.mi or
mi-ChangeLog?
From Peter.Schauer@regent.e-technik.tu-muenchen.de Tue Mar 14 02:46:00 2000
From: "Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de>
To: gdb-patches@sourceware.cygnus.com
Subject: RFA: symfile.c: Fix for GDB crash when rereading symbols
Date: Tue, 14 Mar 2000 02:46:00 -0000
Message-id: <200003141046.LAA08835@reisser.regent.e-technik.tu-muenchen.de>
X-SW-Source: 2000-03/msg00250.html
Content-length: 837

symfile.c:reread_symbols does not clear the new msymbol hash tables in the
objfile, causing stale pointers and a GDB crash during the reread.exp
test on Solaris.

Here is a fix:

	* symfile.c (reread_symbols):  Clear msymbol hash table.

*** gdb/symfile.c.orig	Thu Feb  3 05:14:35 2000
--- gdb/symfile.c	Tue Mar 14 10:00:27 2000
***************
*** 1775,1780 ****
--- 1775,1784 ----
  	      objfile->free_psymtabs = NULL;
  	      objfile->msymbols = NULL;
  	      objfile->minimal_symbol_count = 0;
+ 	      memset (&objfile->msymbol_hash, 0,
+ 		      sizeof (objfile->msymbol_hash));
+ 	      memset (&objfile->msymbol_demangled_hash, 0,
+ 		      sizeof (objfile->msymbol_demangled_hash));
  	      objfile->fundamental_types = NULL;
  	      if (objfile->sf != NULL)
  		{

-- 
Peter Schauer			pes@regent.e-technik.tu-muenchen.de
From ezannoni@cygnus.com Tue Mar 14 09:01:00 2000
From: Elena Zannoni <ezannoni@cygnus.com>
To: Andrew Cagney <ac131313@cygnus.com>
Cc: Elena Zannoni <ezannoni@cygnus.com>, gdb-patches@sourceware.cygnus.com
Subject: Re: PATCH: printing elements of typedef'ed arrays
Date: Tue, 14 Mar 2000 09:01:00 -0000
Message-id: <14542.28768.797041.244558@kwikemart.cygnus.com>
References: <14535.62150.908914.943283@kwikemart.cygnus.com> <38CDED0E.2FBE16B6@cygnus.com>
X-SW-Source: 2000-03/msg00251.html
Content-length: 5335

Andrew Cagney writes:
 > Elena Zannoni wrote:
 > > 
 > > [Sorry, wrong list...]
 > > 
 > > When an array is typedeffed, like in this example:
 > > 
 > > typedef long ArrayLong [10];
 > > ArrayLong a1;
 > > 
 > > typedef struct s
 > > {
 > >  int a;
 > >  int b;
 > > } structure;
 > > 
 > > long a2 [10];
 > > structure s1;
 > > 
 > > int main (void)
 > > {
 > >     return 0;
 > > }
 > > 
 > > Gdb cannot print individual elements of the array a1:
 > > 
 > > (gdb) p a1
 > > $1 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 > > (gdb) p a1[0]
 > > $2 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}  <<<< is incorrect
 > > (gdb) p a2
 > > $3 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0}
 > > (gdb) p a2[0]
 > > $4 = 0
 > > (gdb)
 > > 
 > > The following patch takes care of this.
 > > I tested it on solaris and showed no regressions.
 > > OK to check in?
 > > 
 > > Elena
 > 
 > Just two cross checks:
 > 
 > 	o	the existing testsuite
 > 		checks that the printing of
 > 		character strings doesn't change
 > 		and the patch didn't change the
 > 		behavour
 > 
 > 		(I've a foggy memory about a great
 > 		debate where the printing of strings
 > 		was made special in some way)

Correct. I saw no regressions.
 > 
 > 	o	you've sneekered in a testsuite
 > 		change (to printcmd.exp?) :-)
 > 

Yes, I added some tests.

 > Andrew
 > 
 > PS: I guess I'm the defacto maintainer.

Here is the full patch. I just committed it.

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.130
diff -c -u -r1.130 ChangeLog
--- ChangeLog	2000/03/14 02:37:24	1.130
+++ ChangeLog	2000/03/14 16:56:48
@@ -1,3 +1,8 @@
+2000-03-14  Elena Zannoni  <ezannoni@kwikemart.cygnus.com>
+
+	* eval.c (evaluate_subexp_with_coercion): Add call to
+ 	check_typedef, to handle typedeffed vars correctly.
+
 Mon Mar 13 21:21:41 2000  Andrew Cagney  <cagney@b1.cygnus.com>
 
 	* defs.h (STREQ, STRCMP, STREQN): Document that these macros are
Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.1.1.7
diff -c -u -r1.1.1.7 eval.c
--- eval.c	1999/12/14 01:05:30	1.1.1.7
+++ eval.c	2000/03/14 16:56:49
@@ -1875,7 +1875,7 @@
 	  val =
 	    locate_var_value
 	    (var, block_innermost_frame (exp->elts[pc + 1].block));
-	  return value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (SYMBOL_TYPE (var))),
+	  return value_cast (lookup_pointer_type (TYPE_TARGET_TYPE (check_typedef (SYMBOL_TYPE (var)))),
 			     val);
 	}
       /* FALLTHROUGH */
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.7
diff -c -u -r1.7 ChangeLog
--- ChangeLog	2000/03/14 06:14:07	1.7
+++ ChangeLog	2000/03/14 16:56:52
@@ -1,3 +1,10 @@
+2000-03-14  Elena Zannoni  <ezannoni@kwikemart.cygnus.com>
+
+	* gdb.base/printcmds.c: Add typedeffed arrays.
+
+	* gdb.base/printcmds.exp (test_print_typedef_arrays): New
+ 	procedure to test arrays that are typedef'd.
+
 2000-03-13  James Ingham  <jingham@leda.cygnus.com>
 
 	* lib/gdb.exp: Fix the gdbtk_start routine to correctly find all
Index: testsuite/gdb.base/printcmds.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/printcmds.c,v
retrieving revision 1.1.1.2
diff -c -u -r1.1.1.2 printcmds.c
--- printcmds.c	1999/06/28 16:03:41	1.1.1.2
+++ printcmds.c	2000/03/14 16:56:52
@@ -59,6 +59,12 @@
 /* Single and multidimensional arrays to test access and printing of array
    members. */
 
+typedef int ArrayInt [10];
+ArrayInt a1 = {2,4,6,8,10,12,14,16,18,20};
+
+typedef char ArrayChar [5];
+ArrayChar a2 = {'a','b','c','d','\0'};
+
 int int1dim[12] = {0,1,2,3,4,5,6,7,8,9,10,11};
 int int2dim[3][4] = {{0,1,2,3},{4,5,6,7},{8,9,10,11}};
 int int3dim[2][3][2] = {{{0,1},{2,3},{4,5}},{{6,7},{8,9},{10,11}}};
@@ -97,5 +103,5 @@
   /* Prevent AIX linker from removing variables.  */
   return ctable1[0] + ctable2[0] + int1dim[0] + int2dim[0][0]
     + int3dim[0][0][0] + int4dim[0][0][0][0] + teststring[0] +
-      *parrays -> array1;
+      *parrays -> array1 + a1[0] + a2[0];
 }
Index: testsuite/gdb.base/printcmds.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/printcmds.exp,v
retrieving revision 1.1.1.3
diff -c -u -r1.1.1.3 printcmds.exp
--- printcmds.exp	1999/08/02 23:46:51	1.1.1.3
+++ printcmds.exp	2000/03/14 16:56:52
@@ -552,6 +552,22 @@
 	" = {{{{0, 1}, {2, 3}, {4, 5}}, {{6, 7}, {8, 9}, {10, 11}}}}"
 }
 
+proc test_print_typedef_arrays {} {
+    global gdb_prompt
+
+    gdb_test "set print elements 24" ""
+
+    gdb_test "p a1" \
+	" = {2, 4, 6, 8, 10, 12, 14, 16, 18, 20}"
+    gdb_test "p a1\[0\]" " = 2"
+    gdb_test "p a1\[9\]" " = 20"
+
+    gdb_test "p a2" \
+	" = \"abcd\""
+    gdb_test "p a2\[0\]" " = 97 'a'"
+    gdb_test "p a2\[3\]" " = 100 'd'"
+}
+
 proc test_artificial_arrays {} {
     # Send \026@ instead of just @ in case the kill character is @.
     gdb_test "p int1dim\[0\]\026@2" " = {0, 1}" {p int1dim[0]@2}
@@ -691,6 +707,7 @@
 	test_print_repeats_10
 	test_print_strings
 	test_print_int_arrays
+	test_print_typedef_arrays
 	test_artificial_arrays
 	test_print_char_arrays
 # We used to do the runto main here.