[patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

Hi,

posted as a new thread.

As described in
	cancel: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #4 [Re: [revert] Regression on PowerPC]
	http://sourceware.org/ml/gdb-patches/2012-03/msg00322.html
just ON_STACK had some regressions.

The expectations in that mail were wrong (at least that cleanup/fix is not
required for gdb.cp/gdb2495.exp).

The problem is that the inferior call return pad breakpoint instruction is
never removed even after inferior call finishes.  It is even still visible in
"maintenance info breakpoints".  This does not matter much for AT_ENTRY_POINT
but for ON_STACK it just corrupts stack.

No regressions on
{x86_64,x86_64-m32,i686}-fedora(15-rawhide)/rhel(5-6)-linux-gnu and for
gdbsever non-extended mode.


Thanks,
Jan


gdb/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* dummy-frame.c: Include gdbthread.h.
	(pop_dummy_frame_bpt): New function.
	(pop_dummy_frame): Initialie DUMMY earlier.  Call pop_dummy_frame_bpt.

gdb/testsuite/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
	(maintenance info breakpoints): New tests.

--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -29,6 +29,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "observer.h"
+#include "gdbthread.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
   xfree (dummy);
 }
 
+/* Delete any breakpoint B which is a momentary breakpoint for return from
+   inferior call matching DUMMY_VOIDP.  */
+
+static int
+pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
+{
+  struct dummy_frame *dummy = dummy_voidp;
+
+  if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)
+      && b->thread == pid_to_thread_id (inferior_ptid))
+    delete_breakpoint (b);
+
+  /* Continue the traversal.  */
+  return 0;
+}
+
 /* Pop *DUMMY_PTR, restoring program state to that before the
    frame was created.  */
 
 static void
 pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
-  struct dummy_frame *dummy;
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  restore_infcall_suspend_state (dummy->caller_state);
 
-  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
+  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
 
   /* restore_infcall_control_state frees inf_state,
      all that remains is to pop *dummy_ptr.  */
-  dummy = *dummy_ptr;
   *dummy_ptr = dummy->next;
   xfree (dummy);
 
--- a/gdb/testsuite/gdb.base/call-signal-resume.exp
+++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
@@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
 gdb_test_no_output "set confirm off"
 gdb_test "return" ""
 
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re "call dummy.*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
 # Resume execution, the program should continue without any signal.
 
 gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"

ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

Just that it is pending.
On Fri, 09 Mar 2012 22:00:45 +0100, Jan Kratochvil wrote:
Hi,

posted as a new thread.

As described in
	cancel: [patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #4 [Re: [revert] Regression on PowerPC]
	http://sourceware.org/ml/gdb-patches/2012-03/msg00322.html
just ON_STACK had some regressions.

The expectations in that mail were wrong (at least that cleanup/fix is not
required for gdb.cp/gdb2495.exp).

The problem is that the inferior call return pad breakpoint instruction is
never removed even after inferior call finishes.  It is even still visible in
"maintenance info breakpoints".  This does not matter much for AT_ENTRY_POINT
but for ON_STACK it just corrupts stack.

No regressions on
{x86_64,x86_64-m32,i686}-fedora(15-rawhide)/rhel(5-6)-linux-gnu and for
gdbsever non-extended mode.


Thanks,
Jan


gdb/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* dummy-frame.c: Include gdbthread.h.
	(pop_dummy_frame_bpt): New function.
	(pop_dummy_frame): Initialie DUMMY earlier.  Call pop_dummy_frame_bpt.

gdb/testsuite/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
	(maintenance info breakpoints): New tests.

--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -29,6 +29,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "observer.h"
+#include "gdbthread.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
   xfree (dummy);
 }
 
+/* Delete any breakpoint B which is a momentary breakpoint for return from
+   inferior call matching DUMMY_VOIDP.  */
+
+static int
+pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
+{
+  struct dummy_frame *dummy = dummy_voidp;
+
+  if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)
+      && b->thread == pid_to_thread_id (inferior_ptid))
+    delete_breakpoint (b);
+
+  /* Continue the traversal.  */
+  return 0;
+}
+
 /* Pop *DUMMY_PTR, restoring program state to that before the
    frame was created.  */
 
 static void
 pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
-  struct dummy_frame *dummy;
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  restore_infcall_suspend_state (dummy->caller_state);
 
-  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
+  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
 
   /* restore_infcall_control_state frees inf_state,
      all that remains is to pop *dummy_ptr.  */
-  dummy = *dummy_ptr;
   *dummy_ptr = dummy->next;
   xfree (dummy);
 
--- a/gdb/testsuite/gdb.base/call-signal-resume.exp
+++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
@@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
 gdb_test_no_output "set confirm off"
 gdb_test "return" ""
 
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re "call dummy.*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
 # Resume execution, the program should continue without any signal.
 
 gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Mark Kettenis]

Sorry Jan, but I've completely lost you.  Can we start this discussion
from zero again?

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

On Mon, 26 Mar 2012 21:53:33 +0200, Mark Kettenis wrote:
> Sorry Jan, but I've completely lost you.  Can we start this discussion
> from zero again?

Not sure what to comment on.

This [patch 1/2] is in fact an unrelated, just it is needed for [patch 2/2].
It has a new testcase so it should be clear what is fixed.


For [patch 2/2] it started with the problem description at:
	[patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7
	http://sourceware.org/ml/gdb-patches/2011-12/msg00795.html

There were tried various (dead-end) ways to fix it so [patch 2/2] is another
way how to fix the problem.


Could you be more specific?


Thanks,
Jan

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Mark Kettenis]

> Date: Mon, 26 Mar 2012 22:31:51 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Mon, 26 Mar 2012 21:53:33 +0200, Mark Kettenis wrote:
> > Sorry Jan, but I've completely lost you.  Can we start this discussion
> > from zero again?
> 
> Not sure what to comment on.
> 
> For [patch 2/2] it started with the problem description at:
> 	[patch] Fix gdb.cp/gdb2495.exp regression with gcc-4.7
> 	http://sourceware.org/ml/gdb-patches/2011-12/msg00795.html
> 
> There were tried various (dead-end) ways to fix it so [patch 2/2] is another
> way how to fix the problem.
> 
> Could you be more specific?

Well, I'd prefer not to have to re-read the entire discussion to
review your diff.  But are you saying that you now do think ON_STACK
is the right way to go and I can simply forget about the history?

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

On Mon, 26 Mar 2012 23:45:27 +0200, Mark Kettenis wrote:
> Well, I'd prefer not to have to re-read the entire discussion to
> review your diff.  But are you saying that you now do think ON_STACK
> is the right way to go and I can simply forget about the history?

We need some place without any installed FDE or a place with our own
(unwind-terminating) FDE.  _start was proven is no longer the right place (on
ppc*), PLT stubs are also no longer the right place (with recent ld).

So remains stack or maybe some malloc-allocated memory but from those ON_STACK
seems to be easier.  I had some concerns about security frameworks limiting
stack executability but I have no real world countercase, it seems to be all
handled correctly.

Still despite [patch 1/2] now correctly removes stale breakpoints from stack
in some cases the breakpoints may remain there such as if inferior longjmps
out of the inferior called function.  With ON_STACK inferior functions it is
a real problem, stale inferior call breakpoints were not a problem before.
breakpoint.h:
    /* The breakpoint at the end of a call dummy.  */
    /* FIXME: What if the function we are calling longjmp()s out of
       the call, or the user gets out with the "return" command?  We
       currently have no way of cleaning up the breakpoint in these
       (obscure) situations.  (Probably can solve this by noticing
       longjmp, "return", etc., it's similar to noticing when a
       watchpoint on a local variable goes out of scope (with hardware
       support for watchpoints)).  */
    bp_call_dummy,

The "return" command is implemented in [patch 1/2] (and the comment is not
updated) but retrospectively I see we may need to also handle the out-of-scope
breakpoints.

OTOH I did not implement it because in some cases backtrace fails temporarily
due to some missing CFIs and it would immediately remove such inferior return
breakpoints.  It happens with
	Watchpoint %d deleted because the program has left the block\n\
	in which its expression is valid.\n"),
but a deleted watchpoint is not so serious like deleted inferior call return
breakpoint.

There may be rather some logic to consider a frame X stale if we successfully
unwind next frame, X is not found and X is not inner to the next frame.
But
(a) There were some plans to remove frame_id_inner completely as it breaks
    with -fsplit-stack for example.
(b) bp_watchpoint_scope should be also reworked the same way that time.

Another way is to really handle such stale breakpoints in the longjmp
breakpoint caught by GDB.  I did not investigate more this way.

But TBH longjmp is a hack, one should use proper C++ exceptions instead and
those get caught properly for inferior calls.  These were some of the reasons
I did not go so far to try deal more with longjmps out of inferior calls,
which are bizarre enough on their own.


Thanks,
Jan


gdb/
2012-03-27  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* breakpoint.h (bp_call_dummy): Update the comment.
	* dummy-frame.c: Include gdbthread.h.
	(pop_dummy_frame_bpt): New function.
	(pop_dummy_frame): Initialie DUMMY earlier.  Call pop_dummy_frame_bpt.

gdb/testsuite/
2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Remove momentary breakpoints for completed inferior calls.
	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
	(maintenance info breakpoints): New tests.

--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -95,10 +95,10 @@ enum bptype
 
     /* The breakpoint at the end of a call dummy.  */
     /* FIXME: What if the function we are calling longjmp()s out of
-       the call, or the user gets out with the "return" command?  We
-       currently have no way of cleaning up the breakpoint in these
-       (obscure) situations.  (Probably can solve this by noticing
-       longjmp, "return", etc., it's similar to noticing when a
+       the call?  "return" command is handled by pop_dummy_frame_bpt.
+       We currently have no way of cleaning up the breakpoint in such
+       (obscure) situation.  (Probably can solve this by noticing
+       longjmp, etc., it's similar to noticing when a
        watchpoint on a local variable goes out of scope (with hardware
        support for watchpoints)).  */
     bp_call_dummy,
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -29,6 +29,7 @@
 #include "gdbcmd.h"
 #include "gdb_string.h"
 #include "observer.h"
+#include "gdbthread.h"
 
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
@@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
   xfree (dummy);
 }
 
+/* Delete any breakpoint B which is a momentary breakpoint for return from
+   inferior call matching DUMMY_VOIDP.  */
+
+static int
+pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
+{
+  struct dummy_frame *dummy = dummy_voidp;
+
+  if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)
+      && b->thread == pid_to_thread_id (inferior_ptid))
+    delete_breakpoint (b);
+
+  /* Continue the traversal.  */
+  return 0;
+}
+
 /* Pop *DUMMY_PTR, restoring program state to that before the
    frame was created.  */
 
 static void
 pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
-  struct dummy_frame *dummy;
+  struct dummy_frame *dummy = *dummy_ptr;
+
+  restore_infcall_suspend_state (dummy->caller_state);
 
-  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
+  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
 
   /* restore_infcall_control_state frees inf_state,
      all that remains is to pop *dummy_ptr.  */
-  dummy = *dummy_ptr;
   *dummy_ptr = dummy->next;
   xfree (dummy);
 
--- a/gdb/testsuite/gdb.base/call-signal-resume.exp
+++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
@@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
 gdb_test_no_output "set confirm off"
 gdb_test "return" ""
 
+# Verify there are no remains of the dummy frame.
+gdb_test_no_output "maintenance print dummy-frames"
+set test "maintenance info breakpoints"
+gdb_test_multiple $test $test {
+    -re "call dummy.*\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
 # Resume execution, the program should continue without any signal.
 
 gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Joel Brobecker]

Hello,

Does anyone have any comment on this patch from Jan? It has been
identified as necessary before the release process gets started.

> gdb/
> 2012-03-27  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Remove momentary breakpoints for completed inferior calls.
> 	* breakpoint.h (bp_call_dummy): Update the comment.
> 	* dummy-frame.c: Include gdbthread.h.
> 	(pop_dummy_frame_bpt): New function.
> 	(pop_dummy_frame): Initialie DUMMY earlier.  Call pop_dummy_frame_bpt.
> 
> gdb/testsuite/
> 2012-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Remove momentary breakpoints for completed inferior calls.
> 	* gdb.base/call-signal-resume.exp (maintenance print dummy-frames)
> 	(maintenance info breakpoints): New tests.

I took a look, and nothing really obvious jumped at me. I'd wait a few
more days, and go ahead and commit if you hear no objection.

Just a quick question from me: Why do we need to remove the breakpoint?
Is it just for GDB's benefit (purge the breakpoint from our breakpoint
list), or also for the inferior's benefit.  Given that we are inserting
the breakpoint in scratch memory, it shouldn't make any different to
the inferior, right?

A couple of suggestions on the phrasing of the comment.

Thanks!

> 
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -95,10 +95,10 @@ enum bptype
>  
>      /* The breakpoint at the end of a call dummy.  */
>      /* FIXME: What if the function we are calling longjmp()s out of
> -       the call, or the user gets out with the "return" command?  We
> -       currently have no way of cleaning up the breakpoint in these
> -       (obscure) situations.  (Probably can solve this by noticing
> -       longjmp, "return", etc., it's similar to noticing when a
> +       the call?  "return" command is handled by pop_dummy_frame_bpt.

                  ^^^ The "return" command


> +       We currently have no way of cleaning up the breakpoint in such
> +       (obscure) situation.  (Probably can solve this by noticing
                               ^^^ We can probably solve
                               (also lose the extra parens, I don't think
                               they bring anything, and it will avoid
                               the parens nesting)
> +       longjmp, etc., it's similar to noticing when a
>         watchpoint on a local variable goes out of scope (with hardware
>         support for watchpoints)).  */
>      bp_call_dummy,
> --- a/gdb/dummy-frame.c
> +++ b/gdb/dummy-frame.c
> @@ -29,6 +29,7 @@
>  #include "gdbcmd.h"
>  #include "gdb_string.h"
>  #include "observer.h"
> +#include "gdbthread.h"
>  
>  /* Dummy frame.  This saves the processor state just prior to setting
>     up the inferior function call.  Older targets save the registers
> @@ -108,19 +109,36 @@ remove_dummy_frame (struct dummy_frame **dummy_ptr)
>    xfree (dummy);
>  }
>  
> +/* Delete any breakpoint B which is a momentary breakpoint for return from
> +   inferior call matching DUMMY_VOIDP.  */
> +
> +static int
> +pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
> +{
> +  struct dummy_frame *dummy = dummy_voidp;
> +
> +  if (b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id)
> +      && b->thread == pid_to_thread_id (inferior_ptid))
> +    delete_breakpoint (b);
> +
> +  /* Continue the traversal.  */
> +  return 0;
> +}
> +
>  /* Pop *DUMMY_PTR, restoring program state to that before the
>     frame was created.  */
>  
>  static void
>  pop_dummy_frame (struct dummy_frame **dummy_ptr)
>  {
> -  struct dummy_frame *dummy;
> +  struct dummy_frame *dummy = *dummy_ptr;
> +
> +  restore_infcall_suspend_state (dummy->caller_state);
>  
> -  restore_infcall_suspend_state ((*dummy_ptr)->caller_state);
> +  iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
>  
>    /* restore_infcall_control_state frees inf_state,
>       all that remains is to pop *dummy_ptr.  */
> -  dummy = *dummy_ptr;
>    *dummy_ptr = dummy->next;
>    xfree (dummy);
>  
> --- a/gdb/testsuite/gdb.base/call-signal-resume.exp
> +++ b/gdb/testsuite/gdb.base/call-signal-resume.exp
> @@ -101,6 +101,18 @@ gdb_test "frame $frame_number" ".*"
>  gdb_test_no_output "set confirm off"
>  gdb_test "return" ""
>  
> +# Verify there are no remains of the dummy frame.
> +gdb_test_no_output "maintenance print dummy-frames"
> +set test "maintenance info breakpoints"
> +gdb_test_multiple $test $test {
> +    -re "call dummy.*\r\n$gdb_prompt $" {
> +	fail $test
> +    }
> +    -re "\r\n$gdb_prompt $" {
> +	pass $test
> +    }
> +}
> +
>  # Resume execution, the program should continue without any signal.
>  
>  gdb_test "break stop_two" "Breakpoint \[0-9\]* at .*"

-- 
Joel

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

Hi,

On Mon, 11 Jun 2012 17:21:48 +0200, Joel Brobecker wrote:
> Does anyone have any comment on this patch from Jan? It has been
> identified as necessary before the release process gets started.

I am not sure what to do with it.  I think it is definitely needed as without
this patch gdb.cp/gdb2495.exp regresses which is user visiable.

But it introduces the regression exploited by the testcase below.  Using
infcalls may start to corrupt inferior stack.  It is mentioned in the initial
post.  Maybe "set breakpoint always-inserted on" would fix it but it would be
fragile having a stale breakpoint just relying nobody will ever reinsert it.

###
While there exist some ways how to prevent it - catching any longjmp and
re-checking any infcall breakpoints possibly becoming stale - it can be fooled
even some other ways.  It would also have performance regression, with stale
infcall we would need to stop-and-continue for every longjmp (we do not
stop-and-continue during continue command in such case now).

But such stack re-checking is also not always good.  In some cases we may just
fail to unwind the stack and in such moment GDB would remove the infcall
breakpoint.  But in reality it would not be stale yet.  While one can compare
some addresses in some case I was facing a problem of making arch-specific
assumptions in generic code.

I can try to implement the ### paragraph.  Fedora already runs with this
patchset applied as is as it is less worse than not having it applied.


Thanks,
Jan


gdb/testsuite/
2012-06-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/stale-infcall.c: New file.
	* gdb.base/stale-infcall.exp: New file.

diff --git a/gdb/testsuite/gdb.base/stale-infcall.c b/gdb/testsuite/gdb.base/stale-infcall.c
new file mode 100644
index 0000000..1f5169a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/stale-infcall.c
@@ -0,0 +1,63 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 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 3 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, see <http://www.gnu.org/licenses/>.  */
+
+#include <setjmp.h>
+#include <string.h>
+#include <stdlib.h>
+
+#define BUFSIZE 0x1000
+
+static jmp_buf jmp;
+
+void
+infcall (void)
+{
+  longjmp (jmp, 1);
+}
+
+static void
+run1 (void)
+{
+  char buf[BUFSIZE / 2];
+  int dummy = 0;
+
+  dummy++; /* break-run1 */
+}
+
+static char buf_zero[BUFSIZE];
+
+static void
+run2 (void)
+{
+  char buf[BUFSIZE];
+
+  memset (buf, 0, sizeof (buf));
+
+  if (memcmp (buf, buf_zero, sizeof (buf)) != 0) /* break-run2 */
+    abort (); /* break-fail */
+}
+
+int
+main ()
+{
+  if (setjmp (jmp) == 0)
+    run1 ();
+  else
+    run2 ();
+
+  return 0; /* break-exit */
+}
diff --git a/gdb/testsuite/gdb.base/stale-infcall.exp b/gdb/testsuite/gdb.base/stale-infcall.exp
new file mode 100644
index 0000000..ab7eaf0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/stale-infcall.exp
@@ -0,0 +1,43 @@
+# Copyright (C) 2012 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 3 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, see <http://www.gnu.org/licenses/>.
+
+set testfile stale-infcall
+set srcfile ${testfile}.c
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-run1"]
+gdb_breakpoint [gdb_get_line_number "break-run2"]
+gdb_breakpoint [gdb_get_line_number "break-exit"]
+gdb_breakpoint [gdb_get_line_number "break-fail"]
+
+gdb_continue_to_breakpoint "break-run1" ".* break-run1 .*"
+
+gdb_test "print infcall ()" " break-run2 .*The program being debugged stopped while in a function called from GDB\\..*When the function is done executing, GDB will silently stop\\."
+
+set test "stack corrupted"
+gdb_test_multiple "continue" $test {
+    -re " break-exit .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " break-fail .*\r\n$gdb_prompt $" {
+	fail $test
+    }
+}

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Joel Brobecker]

> I am not sure what to do with it.  I think it is definitely needed as without
> this patch gdb.cp/gdb2495.exp regresses which is user visiable.
> 
> But it introduces the regression exploited by the testcase below.

:-(. I am not sure what to do either. Do you think we could have
a solution in a reasonable amount of time? I don't want to give up
to easily, but it feels like the further we go, the more issue we
find (it feels the same way I felt with the objfile search order
patch series).

We have several options:

  1. Ignore the initial regression, and release with it

  2. Revert the patch that caused the regression. I can't remember
     which patch that was, and whether it would "unsolve" an important
     issue.

  3. Ignore the side-effect/regression caused by this fix, and fix
     it later

  4. Delay the release in order to implement setjmp handling.

Not being completely up to speed with the issues involved, I am
not sure which one would make sense. Delaying the release would
have some impact on Maciej's mips work, but is otherwise a real
possibility.

-- 
Joel

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

On Mon, 11 Jun 2012 21:24:59 +0200, Joel Brobecker wrote:
>   1. Ignore the initial regression, and release with it

This is not good, this regresses a nice C++ fixup by Tom.


>   2. Revert the patch that caused the regression. I can't remember
>      which patch that was, and whether it would "unsolve" an important
>      issue.

The regression is due to binutils fix by Jakub Jelinek:
	http://sourceware.org/bugzilla/show_bug.cgi?id=12570

There is nothing to revert.  The original assumption _start has no unwind info
was just wrong.  Moreover there was a bug that it relied upon the fact that
.plt has no unwind info.  But after fixing it to make it really _start (and
not .plt) some archs (ppc IIRC) has unwind info even for _start.


>   3. Ignore the side-effect/regression caused by this fix, and fix
>      it later

Yes, I find it a viable alternative.


>   4. Delay the release in order to implement setjmp handling.

I already tried to implement it once but somehow did not finish it.  I do no
like much that even the longjmps tracking is not a perfect solution.


It would be best to find some other safe place where to put a breakpoint.
Maybe to override the address choosing in i386-linux-tdep.c and reuse return
address to _start?  Therefore to put breakpoint at 0x7fffffffda58 (+ 7):

Dump of assembler code for function _start:
   0x0000000000489560 <+0>:	xor    %ebp,%ebp
[...]
   0x0000000000489584 <+36>:	callq  0x488180 <__libc_start_main@plt>
=> 0x0000000000489589 <+41>:	hlt    
   0x000000000048958a <+42>:	xchg   %ax,%ax
   0x000000000048958c <+44>:	nopl   0x0(%rax)
(gdb) x/gx 0x7fffffffda58
0x7fffffffda58:	0x0000000000489589


Thanks,
Jan

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

On Mon, 11 Jun 2012 21:40:37 +0200, Jan Kratochvil wrote:
> It would be best to find some other safe place where to put a breakpoint.
> Maybe to override the address choosing in i386-linux-tdep.c and reuse return
> address to _start?  Therefore to put breakpoint at 0x7fffffffda58 (+ 7):

That is also wrong, this would break inferior unwinders before they find the
0 unwind terminator for unwinding from _start.


Jan

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Joel Brobecker]

I took some time to sleep on this...

> >   1. Ignore the initial regression, and release with it
> 
> This is not good, this regresses a nice C++ fixup by Tom.
> 
> 
> >   2. Revert the patch that caused the regression. I can't remember
> >      which patch that was, and whether it would "unsolve" an important
> >      issue.
> 
> The regression is due to binutils fix by Jakub Jelinek:
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=12570
> 
> There is nothing to revert.  The original assumption _start has no unwind info
> was just wrong.  Moreover there was a bug that it relied upon the fact that
> .plt has no unwind info.  But after fixing it to make it really _start (and
> not .plt) some archs (ppc IIRC) has unwind info even for _start.

OK and OK. Thanks for the feedback on these points.

> >   3. Ignore the side-effect/regression caused by this fix, and fix
> >      it later
> 
> Yes, I find it a viable alternative.
> 
> 
> >   4. Delay the release in order to implement setjmp handling.
> 
> I already tried to implement it once but somehow did not finish it.  I do no
> like much that even the longjmps tracking is not a perfect solution.

It seems to me that fixing the problem is going to be quite delicate,
since you keep finding regressions with all the solutions you've been
considering. So I am inclined to accept the regression for 7.5 too.

-- 
Joel

Re: ping: [patch 1/2] Fix gdb.cp/gdb2495.exp regression with gcc-4.7 #5

[Jan Kratochvil]

On Wed, 13 Jun 2012 17:00:44 +0200, Joel Brobecker wrote:
> It seems to me that fixing the problem is going to be quite delicate,
> since you keep finding regressions with all the solutions you've been
> considering. So I am inclined to accept the regression for 7.5 too.

I have the patchset ready, just commenting / cleaning it up before posting it.


Thanks,
Jan