SIGSEGV on gdb 6.7*

SIGSEGV on gdb 6.7*

[Greg Law]

Hi list,

I have been playing with gdb-6.7 and it appears there is a bug whereby 
reading registers can cause a SIGSEGV in gdb.  The simplest way I've 
found to cause the problem is to try to read a register after calling 
the flushregs maintenance command:

$ cat hello.c
#include <stdio.h>

int
main (void)
{
     printf ("hello, world\n");
     return 0;
}
$ gcc -g hello.c
$ ./gdb a.out
GNU gdb 6.7.1
Copyright (C) 2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu"...
Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
(gdb) start
Breakpoint 1 at 0x8048385: file hello.c, line 6.
Starting program: /mnt/duron/tests/a.out
main () at hello.c:6
6           printf ("Hello world\n");
(gdb) flushregs
Register cache flushed.
(gdb) print $pc
Segmentation fault (core dumped)
$


I realise that flushregs is not exactly a "supported" feature, but since 
the implementation of that command is simply:

static void
reg_flush_command (char *command, int from_tty)
{
   /* Force-flush the register cache.  */
   registers_changed ();
   if (from_tty)
     printf_filtered (_("Register cache flushed.\n"));
}

one wonders if there aren't other ways this issue can be triggered.

I have had a look around the code, and it's one of those "how could it 
ever have worked?" occasions (which of course typically means I've 
missed something important).  The implementation of registers_changed() 
is (essentially):

void
registers_changed (void)
{
   int i;

   regcache_xfree (current_regcache);
   current_regcache = NULL;

yet there appear to be pointers to the regcache squirrelled away in 
various other places, notably the prologue_cache member of the 
frame_info structure.

I'm sure I'm missing something here -- how come freeing current_regcache 
is safe with pointers to the register cache out-living it?

I have found that the following simple patch appears to fix (or at least 
work round) the SEGV triggered by flushregs:

Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.163
diff -u -r1.163 regcache.c
--- regcache.c  1 Jan 2008 22:53:12 -0000       1.163
+++ regcache.c  3 Feb 2008 21:35:19 -0000
@@ -885,6 +885,7 @@
  {
    /* Force-flush the register cache.  */
    registers_changed ();
+  get_current_regcache ();
    if (from_tty)
      printf_filtered (_("Register cache flushed.\n"));
  }

However, I am insufficiently familiar with the gdb internals to know 
which, if any, of the other call-sites to register_changed() should be 
similarly updated.  Perhaps it would be safer simply to call 
get_current_regcache() from the end of registers_changed():

Index: regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.163
diff -u -r1.163 regcache.c
--- regcache.c  1 Jan 2008 22:53:12 -0000       1.163
+++ regcache.c  3 Feb 2008 21:49:37 -0000
@@ -472,6 +472,9 @@
    regcache_xfree (current_regcache);
    current_regcache = NULL;

+  /* Update the cache with the new registers. */
+  get_current_regcache ();
+
    /* Force cleanup of any alloca areas if using C alloca instead of
       a builtin alloca.  This particular call is used to clean up
       areas allocated by low level target code which may build up


I guess this might result in some "unnecessary" fetches of the register 
state, but that has to be favourable to a SEGV :)

Cheers,

Greg


-- 
Greg Law, Undo Software                       http://undo-software.com/

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Mon, Feb 04, 2008 at 08:49:48PM +0000, Greg Law wrote:
> yet there appear to be pointers to the regcache squirrelled away in  
> various other places, notably the prologue_cache member of the frame_info 
> structure.

Could you give a specific example?  I don't think there should be such
pointers.

> I guess this might result in some "unnecessary" fetches of the register  
> state, but that has to be favourable to a SEGV :)

Not really - we go to a lot of trouble to avoid this.

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Greg Law]

Daniel Jacobowitz wrote:
> On Mon, Feb 04, 2008 at 08:49:48PM +0000, Greg Law wrote:
>> yet there appear to be pointers to the regcache squirrelled away in  
>> various other places, notably the prologue_cache member of the 
>> frame_info structure.
>
> Could you give a specific example?  I don't think there should be such
> pointers.

When a register is examined, we (eventually) get to frame_register_unwind.  
This does:

frame->unwind->prev_register (frame->next, &frame->prologue_cache, regnum,
                optimizedp, lvalp, addrp, realnump, bufferp);

which is actually a function pointer to (on plain old x86 Linux) 
sentinel_frame_prev_register, which goes:

static void
sentinel_frame_prev_register (struct frame_info *next_frame,
                  void **this_prologue_cache,
                  int regnum, int *optimized,
                  enum lval_type *lvalp, CORE_ADDR *addrp,
                  int *realnum, gdb_byte *bufferp)
{
 struct frame_unwind_cache *cache = *this_prologue_cache;
 /* Describe the register's location.  A reg-frame maps all registers
    onto the corresponding hardware register.  */
 *optimized = 0;
 *lvalp = lval_register;
 *addrp = register_offset_hack (current_gdbarch, regnum);
 *realnum = regnum;

 /* If needed, find and return the value of the register.  */
 if (bufferp != NULL)
   {
     /* Return the actual value.  */
     /* Use the regcache_cooked_read() method so that it, on the fly,
        constructs either a raw or pseudo register from the raw
        register cache.  */
     regcache_cooked_read (cache->regcache, regnum, bufferp);
   }
}

note the first line which is an assignment to 'cache' from
*this_prologue_cache.  The regcache_cooked_read is called, passing 
cache->regcache.  If flushregs has been called, the regcache appears 
to be garbage.  If I've not got myself terribly confused, this is 
not especially surprising, since we freed the regcache at
registers_changed(), leaving the frame_info structure pointing at
invalid memory.

>
>> I guess this might result in some "unnecessary" fetches of the register  
>> state, but that has to be favourable to a SEGV :)
>
> Not really - we go to a lot of trouble to avoid this.

Yeah, I guess this more important on remote targets over slow links, 
right?  In that case, do we not need to take care from 
registers_changed() to go and invalidate all the pointers to cached 
registers in the frame structures?  Hmm, although thinking about it 
now, I guess the frame structures all need throwing away/recalculating 
if the registers have changed anyway?  I can't find what causes that 
to happen though.

g

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

[Please reply to the list, thanks!]

On Mon, Feb 04, 2008 at 09:35:07PM +0000, Greg Law wrote:
> When a register is examined, we (eventually) get to  
> frame_register_unwind.  This does:
>
> frame->unwind->prev_register (frame->next, &frame->prologue_cache, regnum,
> 				optimizedp, lvalp, addrp, realnump, bufferp);
>
> which is actually a function pointer to (on plain old x86 Linux) 
> sentinel_frame_prev_register, which goes:

Ah yes.  That's the only prologue cache which has any business
accessing a regcache directly, none of the others do.

flushregs should invalidate the frame cache and current/selected
frame.  I have been meaning to fix that for, roughly, ever.  Does
it work better if you do that?

> Yeah, I guess this more important on remote targets over slow
> links, right?  In that case, do we not need to take care from
> registers_changed() to go and invalidate all the pointers to cached 
> registers in the frame structures?  Hmm, although thinking about it now, I 
> guess the frame structures all need
> throwing away/recalculating if the registers have changed anyway?  I can't 
> find what causes that to happen though.

That's the key point :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Greg Law]

Daniel Jacobowitz wrote:
> [Please reply to the list, thanks!]
> 
> On Mon, Feb 04, 2008 at 09:35:07PM +0000, Greg Law wrote:
>> When a register is examined, we (eventually) get to  
>> frame_register_unwind.  This does:
>>
>> frame->unwind->prev_register (frame->next, &frame->prologue_cache, regnum,
>> 				optimizedp, lvalp, addrp, realnump, bufferp);
>>
>> which is actually a function pointer to (on plain old x86 Linux) 
>> sentinel_frame_prev_register, which goes:
> 
> Ah yes.  That's the only prologue cache which has any business
> accessing a regcache directly, none of the others do.
> 
> flushregs should invalidate the frame cache and current/selected
> frame.  I have been meaning to fix that for, roughly, ever.  Does
> it work better if you do that?

Should it be just flushregs that invalidates the frame cache, or should 
it happen from registers_changed()?

g
-- 
Greg Law, Undo Software                       http://undo-software.com/

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Mon, Feb 04, 2008 at 09:58:42PM +0000, Greg Law wrote:
> Should it be just flushregs that invalidates the frame cache, or should  
> it happen from registers_changed()?

Good question :-)  Probably registers_changed, but I suspect that
makes a lot of other reinit_frame_cache calls redundant.

In the long term there should be more than a single register cache,
and more than a single frame cache - one per thread, for instance.

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Greg Law]

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

Daniel Jacobowitz wrote:
> On Mon, Feb 04, 2008 at 09:58:42PM +0000, Greg Law wrote:
>> Should it be just flushregs that invalidates the frame cache, or should  
>> it happen from registers_changed()?
> 
> Good question :-)  Probably registers_changed, but I suspect that
> makes a lot of other reinit_frame_cache calls redundant.
> 

Well, calling reinit_frame_cache() from registers_changed() does indeed 
fix the problem.  It doesn't seem that a few redundant calls to 
reinit_frame_cache() is a big issue: if it's already NULL then there 
isn't much work for that function to do.  OTOH, not calling it where it 
should be called clearly is pretty serious.

The attached patch does this.  I guess copyright assignment etc is 
rather over the top for such a small change, but if you prefer we go 
through those hoops then I'm happy to do so.

g

-- 
Greg Law, Undo Software                       http://undo-software.com/

[-- Attachment #2: regcache_fix.patch --]
[-- Type: text/x-patch, Size: 658 bytes --]

Index: gdb/regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.163
diff -u -r1.163 regcache.c
--- gdb/regcache.c	1 Jan 2008 22:53:12 -0000	1.163
+++ gdb/regcache.c	4 Feb 2008 22:24:32 -0000
@@ -472,6 +472,9 @@
   regcache_xfree (current_regcache);
   current_regcache = NULL;
 
+  /* Need to forget about any frames we have cached, too. */
+  reinit_frame_cache ();
+
   /* Force cleanup of any alloca areas if using C alloca instead of
      a builtin alloca.  This particular call is used to clean up
      areas allocated by low level target code which may build up

Re: SIGSEGV on gdb 6.7*

[Greg Law]

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

Hi Daniel,

Any chance this patch (or its equivalent) could get applied?

(Attached again for the sake of convenience.)

Cheers,

Greg

Greg Law wrote:
> Daniel Jacobowitz wrote:
>> On Mon, Feb 04, 2008 at 09:58:42PM +0000, Greg Law wrote:
>>> Should it be just flushregs that invalidates the frame cache, or 
>>> should  it happen from registers_changed()?
>>
>> Good question :-)  Probably registers_changed, but I suspect that
>> makes a lot of other reinit_frame_cache calls redundant.
>>
> 
> Well, calling reinit_frame_cache() from registers_changed() does indeed 
> fix the problem.  It doesn't seem that a few redundant calls to 
> reinit_frame_cache() is a big issue: if it's already NULL then there 
> isn't much work for that function to do.  OTOH, not calling it where it 
> should be called clearly is pretty serious.
> 
> The attached patch does this.  I guess copyright assignment etc is 
> rather over the top for such a small change, but if you prefer we go 
> through those hoops then I'm happy to do so.
> 
> g
> 


-- 
Greg Law, Undo Software                       http://undo-software.com/

[-- Attachment #2: regcache_fix.patch --]
[-- Type: text/x-patch, Size: 658 bytes --]

Index: gdb/regcache.c
===================================================================
RCS file: /cvs/src/src/gdb/regcache.c,v
retrieving revision 1.163
diff -u -r1.163 regcache.c
--- gdb/regcache.c	1 Jan 2008 22:53:12 -0000	1.163
+++ gdb/regcache.c	4 Feb 2008 22:24:32 -0000
@@ -472,6 +472,9 @@
   regcache_xfree (current_regcache);
   current_regcache = NULL;
 
+  /* Need to forget about any frames we have cached, too. */
+  reinit_frame_cache ();
+
   /* Force cleanup of any alloca areas if using C alloca instead of
      a builtin alloca.  This particular call is used to clean up
      areas allocated by low level target code which may build up

Re: SIGSEGV on gdb 6.7*

[Michael Snyder]

On Mon, 2008-02-04 at 20:49 +0000, Greg Law wrote:
> Hi list,
> 
> I have been playing with gdb-6.7 and it appears there is a bug whereby 
> reading registers can cause a SIGSEGV in gdb.  The simplest way I've 
> found to cause the problem is to try to read a register after calling 
> the flushregs maintenance command:

First of all, I apologize for not joining this thread sooner --
it escaped my notice.

Secondly, I apologize because I wrote the flushregs 
maintenance command, and it is clearly wrong.  I actually
had the impression that I HAD made it flush the frame 
cache -- I certainly meant to.

That said -- I agree with Daniel.  I can see where 
flushing the register cache and flushing the frame cache
are two things that should probably always be done at
the same time -- but I'm worried about the extra overhead
that this patch introduces.  We call registers_changed
A LOT, and in doing so we assume that it has a very
low overhead.

I would prefer to just fix the reg_flush_command function
for now, and return to the question "should registers_changed
imply reinit_frame_cache (and vice versa) as an architectural
question later (or sooner).

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Tue, Feb 26, 2008 at 02:54:47PM -0800, Michael Snyder wrote:
> That said -- I agree with Daniel.  I can see where 
> flushing the register cache and flushing the frame cache
> are two things that should probably always be done at
> the same time -- but I'm worried about the extra overhead
> that this patch introduces.  We call registers_changed
> A LOT, and in doing so we assume that it has a very
> low overhead.

If the registers have changed, how can the frame cache still possibly
be valid?

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Michael Snyder]

On Tue, 2008-02-26 at 19:26 -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 26, 2008 at 02:54:47PM -0800, Michael Snyder wrote:
> > That said -- I agree with Daniel.  I can see where 
> > flushing the register cache and flushing the frame cache
> > are two things that should probably always be done at
> > the same time -- but I'm worried about the extra overhead
> > that this patch introduces.  We call registers_changed
> > A LOT, and in doing so we assume that it has a very
> > low overhead.
> 
> If the registers have changed, how can the frame cache still possibly
> be valid?

No argument -- it can't.

Are you swinging around toward wanting to accept this patch?
;-)

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Tue, Feb 26, 2008 at 04:37:33PM -0800, Michael Snyder wrote:
> On Tue, 2008-02-26 at 19:26 -0500, Daniel Jacobowitz wrote:
> > On Tue, Feb 26, 2008 at 02:54:47PM -0800, Michael Snyder wrote:
> > > That said -- I agree with Daniel.  I can see where 
> > > flushing the register cache and flushing the frame cache
> > > are two things that should probably always be done at
> > > the same time -- but I'm worried about the extra overhead
> > > that this patch introduces.  We call registers_changed
> > > A LOT, and in doing so we assume that it has a very
> > > low overhead.
> > 
> > If the registers have changed, how can the frame cache still possibly
> > be valid?
> 
> No argument -- it can't.
> 
> Are you swinging around toward wanting to accept this patch?
> ;-)

I'm talking about Greg's version, which calls it from registers_changed.
What do you think of that one?

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Michael Snyder]

On Tue, 2008-02-26 at 19:43 -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 26, 2008 at 04:37:33PM -0800, Michael Snyder wrote:
> > On Tue, 2008-02-26 at 19:26 -0500, Daniel Jacobowitz wrote:
> > > On Tue, Feb 26, 2008 at 02:54:47PM -0800, Michael Snyder wrote:
> > > > That said -- I agree with Daniel.  I can see where 
> > > > flushing the register cache and flushing the frame cache
> > > > are two things that should probably always be done at
> > > > the same time -- but I'm worried about the extra overhead
> > > > that this patch introduces.  We call registers_changed
> > > > A LOT, and in doing so we assume that it has a very
> > > > low overhead.
> > > 
> > > If the registers have changed, how can the frame cache still possibly
> > > be valid?
> > 
> > No argument -- it can't.
> > 
> > Are you swinging around toward wanting to accept this patch?
> > ;-)
> 
> I'm talking about Greg's version, which calls it from registers_changed.

Yeah, me too.  This is that thread.  ;-)

> What do you think of that one?

I think it's probably the right thing to do, I'm must
a little concerned about the overhead.  I wouldn't oppose
putting it in, I just thought we could benefit from a little
breathing room to think about it.

The only case where we know it will crash is reg_flush_command, 
and in that case, flushing the frames is unquestionably desirable.
So if we fix it there, we cover the known crasher, and then we 
have breathing space to contemplate the more general fix.

Or, we can just accept Greg's patch.

Back to you... what's your inclination?   ;-)

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Tue, Feb 26, 2008 at 04:51:33PM -0800, Michael Snyder wrote:
> Back to you... what's your inclination?   ;-)

OK, let's take it.  I'll check it in to HEAD in a couple of minutes.

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Michael Snyder]

On Tue, 2008-02-26 at 19:54 -0500, Daniel Jacobowitz wrote:
> On Tue, Feb 26, 2008 at 04:51:33PM -0800, Michael Snyder wrote:
> > Back to you... what's your inclination?   ;-)
> 
> OK, let's take it.  I'll check it in to HEAD in a couple of minutes.

A result that should be satisfactory to all.

Re: SIGSEGV on gdb 6.7*

[Greg Law]

Michael Snyder wrote:
> On Tue, 2008-02-26 at 19:54 -0500, Daniel Jacobowitz wrote:
>> On Tue, Feb 26, 2008 at 04:51:33PM -0800, Michael Snyder wrote:
>>> Back to you... what's your inclination?   ;-)
>> OK, let's take it.  I'll check it in to HEAD in a couple of minutes.
> 
> A result that should be satisfactory to all.

Thanks guys :)

g
-- 
Greg Law, Undo Software                       http://undo-software.com/

Re: SIGSEGV on gdb 6.7*

[Vladimir Prus]

Greg Law wrote:

> Michael Snyder wrote:
>> On Tue, 2008-02-26 at 19:54 -0500, Daniel Jacobowitz wrote:
>>> On Tue, Feb 26, 2008 at 04:51:33PM -0800, Michael Snyder wrote:
>>>> Back to you... what's your inclination?   ;-)
>>> OK, let's take it.  I'll check it in to HEAD in a couple of minutes.
>> 
>> A result that should be satisfactory to all.
> 
> Thanks guys :)

This patch cause testsuite regressions for me, as follows:

        FAIL: gdb.base/annota1.exp: continue to printf
        FAIL: gdb.base/annota1.exp: send SIGUSR1
        FAIL: gdb.base/annota1.exp: signal sent
        FAIL: gdb.cp/annota2.exp: continue to exit
        FAIL: gdb.cp/annota2.exp: watch triggered on a.x

This is Kubuntu 7.10 Gutsy, and gcc version is:

        gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)

JFYI -- I'm not sure we should bother about annotations.

- Volodya

Re: SIGSEGV on gdb 6.7*

[Thiago Jung Bauermann]

On Thu, 2008-02-28 at 10:15 +0300, Vladimir Prus wrote:
> This patch cause testsuite regressions for me, as follows:
> 
>         FAIL: gdb.base/annota1.exp: continue to printf
>         FAIL: gdb.base/annota1.exp: send SIGUSR1
>         FAIL: gdb.base/annota1.exp: signal sent

In my experience, this testcase is "nondeterministic", meaning that it
will go back and forth between PASS and FAIL for no good reason.

I keep a list of testcases which I see that have such behaviour:

* gdb.base/annota1.exp 
  * gdb.base/annota2.exp 
  * gdb.base/annota3.exp 
  * gdb.base/checkpoint.exp 
  * gdb.base/display.exp (observed in GDB 6.5, seems fine in 6.6) 
  * gdb.base/interrupt.exp (observed in GDB 6.5, seems fine in 6.6) 
  * gdb.base/multi-forks.exp 
  * gdb.threads/attachstop.exp (observed in GDB 6.5). Test dies with: 
        ERROR: couldn't open "/proc/28696/status": no such file or directory
  * gdb.threads/schedlock.exp 
  * gdb.threads/watchthreads2.exp

Is it useful to have such list on the wiki?
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 11:12:41AM -0300, Thiago Jung Bauermann wrote:
> On Thu, 2008-02-28 at 10:15 +0300, Vladimir Prus wrote:
> > This patch cause testsuite regressions for me, as follows:
> > 
> >         FAIL: gdb.base/annota1.exp: continue to printf
> >         FAIL: gdb.base/annota1.exp: send SIGUSR1
> >         FAIL: gdb.base/annota1.exp: signal sent
> 
> In my experience, this testcase is "nondeterministic", meaning that it
> will go back and forth between PASS and FAIL for no good reason.

The whole testcase is not; there's a few particular subtests that are,
which involve SIGINT.  We've never managed to reproduce that one
outside of expect, unfortunately.

> Is it useful to have such list on the wiki?

Sure, but with attached example logs might be best so that we can
remove them when they're fixed.

-- 
Daniel Jacobowitz
CodeSourcery

Re: SIGSEGV on gdb 6.7*

[Daniel Jacobowitz]

On Thu, Feb 28, 2008 at 10:15:35AM +0300, Vladimir Prus wrote:
> This patch cause testsuite regressions for me, as follows:
> 
>         FAIL: gdb.base/annota1.exp: continue to printf
>         FAIL: gdb.base/annota1.exp: send SIGUSR1
>         FAIL: gdb.base/annota1.exp: signal sent
>         FAIL: gdb.cp/annota2.exp: continue to exit
>         FAIL: gdb.cp/annota2.exp: watch triggered on a.x
> 
> This is Kubuntu 7.10 Gutsy, and gcc version is:
> 
>         gcc version 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2)
> 
> JFYI -- I'm not sure we should bother about annotations.

My fault.  These are easy to fix, as attached.  I'll check this in shortly.

-- 
Daniel Jacobowitz
CodeSourcery

2008-02-28  Daniel Jacobowitz  <dan@codesourcery.com>

	* frame.c (reinit_frame_cache): Only annotate if frames were
	previously valid.

2008-02-28  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.base/annota1.exp, gdb.cp/annota2.exp: Update for fewer
	frames-invalid annotations.

Index: frame.c
===================================================================
RCS file: /cvs/src/src/gdb/frame.c,v
retrieving revision 1.236
diff -u -p -r1.236 frame.c
--- frame.c	1 Jan 2008 22:53:09 -0000	1.236
+++ frame.c	28 Feb 2008 14:15:46 -0000
@@ -1079,9 +1079,11 @@ reinit_frame_cache (void)
   obstack_free (&frame_cache_obstack, 0);
   obstack_init (&frame_cache_obstack);
 
+  if (current_frame != NULL)
+    annotate_frames_invalid ();
+
   current_frame = NULL;		/* Invalidate cache */
   select_frame (NULL);
-  annotate_frames_invalid ();
   if (frame_debug)
     fprintf_unfiltered (gdb_stdlog, "{ reinit_frame_cache () }\n");
 }
Index: testsuite/gdb.base/annota1.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/annota1.exp,v
retrieving revision 1.27
diff -u -p -r1.27 annota1.exp
--- testsuite/gdb.base/annota1.exp	26 Feb 2008 08:14:11 -0000	1.27
+++ testsuite/gdb.base/annota1.exp	28 Feb 2008 14:15:46 -0000
@@ -149,7 +149,7 @@ gdb_expect {
 set binexp [string_to_regexp $binfile]
 send_gdb "run\n"
 gdb_expect {
-    -re "\r\n\032\032post-prompt\r\nStarting program: $binexp \(\r\n\r\n\032\032frames-invalid\)+\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)*\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)*\r\n\r\n\032\032breakpoint 1\r\n\r\nBreakpoint 1, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*annota1.c\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$main_line\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*$srcfile:$main_line:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped.*$gdb_prompt$" {
+    -re "\r\n\032\032post-prompt\r\nStarting program: $binexp \(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)+\r\n\r\n\032\032starting\(\(\r\n\r\n\032\032frames-invalid\)|\(\r\n\r\n\032\032breakpoints-invalid\)\)*\r\n\r\n\032\032breakpoint 1\r\n\r\nBreakpoint 1, \r\n\032\032frame-begin 0 $hex\r\n\r\n\032\032frame-function-name\r\nmain\r\n\032\032frame-args\r\n \\(\\)\r\n\032\032frame-source-begin\r\n at \r\n\032\032frame-source-file\r\n.*annota1.c\r\n\032\032frame-source-file-end\r\n:\r\n\032\032frame-source-line\r\n$main_line\r\n\032\032frame-source-end\r\n\r\n\r\n\032\032source.*$srcfile:$main_line:.*:beg:$hex\r\n\r\n\032\032frame-end\r\n\r\n\032\032stopped.*$gdb_prompt$" {
 	pass "run until main breakpoint" 
     }
     -re ".*$gdb_prompt$" { 
@@ -466,7 +466,7 @@ if [target_info exists gdb,nosignals] {
     setup_xfail hppa*-*-hpux11*
     send_gdb "signal SIGTRAP\n"
     gdb_expect {
-      -re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
+      -re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
 	      { pass "signal sent" }
       -re ".*$gdb_prompt$" { fail "signal sent" }
       timeout { fail "signal sent (timeout)" }
Index: testsuite/gdb.cp/annota2.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/annota2.exp,v
retrieving revision 1.9
diff -u -p -r1.9 annota2.exp
--- testsuite/gdb.cp/annota2.exp	1 Jan 2008 22:53:19 -0000	1.9
+++ testsuite/gdb.cp/annota2.exp	28 Feb 2008 14:15:46 -0000
@@ -122,7 +122,7 @@ gdb_expect {
 #
 send_gdb "continue\n"
 gdb_expect {
-  -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\na.x is 1\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032exited 0\r\n\r\nProgram exited normally.\r\n\r\n\032\032frames-invalid\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
+  -re "\r\n\032\032post-prompt\r\nContinuing.\r\n\r\n\032\032starting\r\n\r\n\032\032frames-invalid\r\na.x is 1\r\n\r\n\032\032exited 0\r\n\r\nProgram exited normally.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" \
 	  { pass "continue until exit" }
   -re ".*$gdb_prompt$"     { fail "continue to exit" }
   timeout	            { fail "continue to exit (timeout)" }

Re: SIGSEGV on gdb 6.7*

[Joel Brobecker]

> On Tue, 2008-02-26 at 19:54 -0500, Daniel Jacobowitz wrote:
> > On Tue, Feb 26, 2008 at 04:51:33PM -0800, Michael Snyder wrote:
> > > Back to you... what's your inclination?   ;-)
> > 
> > OK, let's take it.  I'll check it in to HEAD in a couple of minutes.
> 
> A result that should be satisfactory to all.

As discussed previously, I checked this patch in the 6.8 branch
as well. That was:

2008-02-26  Greg Law  <glaw@undo-software.com>

        * regcache.c (registers_changed): Call reinit_frame_cache.

And apparently, this was causing some failures in the testsuite
which Daniel quickly fixed. So I also applied:

2008-02-28  Daniel Jacobowitz  <dan@codesourcery.com>

        * frame.c (reinit_frame_cache): Only annotate if frames were
        previously valid.

2008-02-28  Daniel Jacobowitz  <dan@codesourcery.com>

        * gdb.base/annota1.exp, gdb.cp/annota2.exp: Update for fewer
        frames-invalid annotations.

(which looked pretty safe).

-- 
Joel