public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* RFC: fix bug with std::terminate handler
@ 2010-02-25 18:11 Tom Tromey
  2010-02-25 18:26 ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2010-02-25 18:11 UTC (permalink / raw)
  To: gdb-patches

I would appreciate comments on this patch.

This comes from an automatically-reported bug in the Red Hat bugzilla:

    https://bugzilla.redhat.com/show_bug.cgi?id=562975

call_function_by_hand installs a momentary breakpoint on std::terminate,
and then deletes it later.  However, this can cause a double deletion of
the breakpoint.  In the bug, the called function is dlopen, which causes
gdb to enter solib_add, which calls breakpoint_re_set, deleting the
momentary breakpoint.

This fix works by creating the momentary breakpoint with an internal
breakpoint number, and then trying to delete the breakpoint by number.

This bug does not always manifest in a crash.  In fact, I couldn't make
it crash here, but I could observe the problem under valgrind.

Built and regtested on x86-64 (compile farm).  I also manually verified
it using valgrind.

I think this patch is mildly ugly, due to the introduction of
set_momentary_breakpoint_at_pc_with_number.  However, in the absence of
comments, I plan to check it in after a reasonable waiting period.

Tom

2010-02-25  Tom Tromey  <tromey@redhat.com>

	* infcall.c (do_delete_breakpoint_by_number): New function.
	(call_function_by_hand): Refer to momentary breakpoint by number.
	* breakpoint.h (set_momentary_breakpoint_at_pc_with_number):
	Declare.
	* breakpoint.c (set_momentary_breakpoint_at_pc_with_number): New
	function.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8c97949..58c590d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6010,6 +6010,20 @@ set_momentary_breakpoint_at_pc (struct gdbarch *gdbarch, CORE_ADDR pc,
 
   return set_momentary_breakpoint (gdbarch, sal, null_frame_id, type);
 }
+
+/* Like set_momentary_breakpoint_at_pc, but ensure that the new
+   breakpoint has a number.  */
+
+struct breakpoint *
+set_momentary_breakpoint_at_pc_with_number (struct gdbarch *gdbarch,
+					    CORE_ADDR pc,
+					    enum bptype type)
+{
+  struct breakpoint *result = set_momentary_breakpoint_at_pc (gdbarch, pc,
+							      type);
+  result->number = internal_breakpoint_number--;
+  return result;
+}
 \f
 
 /* Tell the user we have just set a breakpoint B.  */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 6b373a3..cf04e2e 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -760,6 +760,9 @@ extern struct breakpoint *set_momentary_breakpoint
 extern struct breakpoint *set_momentary_breakpoint_at_pc
   (struct gdbarch *, CORE_ADDR pc, enum bptype type);
 
+extern struct breakpoint *set_momentary_breakpoint_at_pc_with_number
+  (struct gdbarch *, CORE_ADDR pc, enum bptype type);
+
 extern struct breakpoint *clone_momentary_breakpoint (struct breakpoint *bpkt);
 
 extern void set_ignore_count (int, int, int);
diff --git a/gdb/infcall.c b/gdb/infcall.c
index e642894..e020233 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -402,6 +402,18 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
   return e;
 }
 
+/* A cleanup function that deletes a breakpoint, if it still exists,
+   given the breakpoint's number.  */
+
+static void
+do_delete_breakpoint_by_number (void *arg)
+{
+  int *num = arg;
+  struct breakpoint *bp = get_breakpoint (*num);
+  if (bp)
+    delete_breakpoint (bp);
+}
+
 /* All this stuff with a dummy frame may seem unnecessarily complicated
    (why not just save registers in GDB?).  The purpose of pushing a dummy
    frame which looks just like a real frame is so that if you call a
@@ -439,7 +451,8 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   struct cleanup *args_cleanup;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
-  struct breakpoint *terminate_bp = NULL;
+  int terminate_bp_num = 0;
+  CORE_ADDR terminate_bp_addr = 0;
   struct minimal_symbol *tm;
   struct cleanup *terminate_bp_cleanup = NULL;
   ptid_t call_thread_ptid;
@@ -758,8 +771,13 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
        struct minimal_symbol *tm = lookup_minimal_symbol  ("std::terminate()",
 							   NULL, NULL);
        if (tm != NULL)
-	   terminate_bp = set_momentary_breakpoint_at_pc
+	 {
+	   struct breakpoint *bp;
+	   bp = set_momentary_breakpoint_at_pc_with_number
 	     (gdbarch, SYMBOL_VALUE_ADDRESS (tm),  bp_breakpoint);
+	   terminate_bp_num = bp->number;
+	   terminate_bp_addr = bp->loc->address;
+	 }
      }
 
   /* Everything's ready, push all the info needed to restore the
@@ -773,8 +791,9 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   discard_cleanups (inf_status_cleanup);
 
   /* Register a clean-up for unwind_on_terminating_exception_breakpoint.  */
-  if (terminate_bp)
-    terminate_bp_cleanup = make_cleanup_delete_breakpoint (terminate_bp);
+  if (terminate_bp_num != 0)
+    terminate_bp_cleanup = make_cleanup (do_delete_breakpoint_by_number,
+					 &terminate_bp_num);
 
   /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
      If you're looking to implement asynchronous dummy-frames, then
@@ -940,9 +959,9 @@ When the function is done executing, GDB will silently stop."),
 		 in an inferior function call. Rewind, and warn the
 		 user.  */
 
-	      if (terminate_bp != NULL
+	      if (terminate_bp_num != 0
 		  && (inferior_thread ()->stop_bpstat->breakpoint_at->address
-		      == terminate_bp->loc->address))
+		      == terminate_bp_addr))
 		{
 		  /* We must get back to the frame we were before the
 		     dummy call.  */
@@ -991,7 +1010,7 @@ When the function is done executing, GDB will silently stop."),
 
   /* If we get here and the std::terminate() breakpoint has been set,
      it has to be cleaned manually.  */
-  if (terminate_bp)
+  if (terminate_bp_num != 0)
     do_cleanups (terminate_bp_cleanup);
 
   /* If we get here the called FUNCTION ran to completion,

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-02-25 18:11 RFC: fix bug with std::terminate handler Tom Tromey
@ 2010-02-25 18:26 ` Pedro Alves
  2010-02-25 19:19   ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2010-02-25 18:26 UTC (permalink / raw)
  To: gdb-patches, tromey

On Thursday 25 February 2010 18:11:44, Tom Tromey wrote:
> call_function_by_hand installs a momentary breakpoint on std::terminate,
> and then deletes it later.  However, this can cause a double deletion of
> the breakpoint.  In the bug, the called function is dlopen, which causes
> gdb to enter solib_add, which calls breakpoint_re_set, deleting the
> momentary breakpoint.

Why doesn't stepping over "dlopen", with "(gdb) next" cause the
same problem with step-resume breakpoints?

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-02-25 18:26 ` Pedro Alves
@ 2010-02-25 19:19   ` Tom Tromey
  2010-02-25 19:49     ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2010-02-25 19:19 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Tom> call_function_by_hand installs a momentary breakpoint on std::terminate,
Tom> and then deletes it later.  However, this can cause a double deletion of
Tom> the breakpoint.  In the bug, the called function is dlopen, which causes
Tom> gdb to enter solib_add, which calls breakpoint_re_set, deleting the
Tom> momentary breakpoint.

Pedro> Why doesn't stepping over "dlopen", with "(gdb) next" cause the
Pedro> same problem with step-resume breakpoints?

The step-resume breakpoints have type bp_step_resume, which are handled
specially by breakpoint_re_set_one.  The std::terminate breakpoint is
just a plain bp_breakpoint.

I suppose I could add a new bptype, though that seems rather heavy for
this.

If you have a particular approach you think would be best, I would be
happy to implement that.

Tom

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-02-25 19:19   ` Tom Tromey
@ 2010-02-25 19:49     ` Pedro Alves
  2010-02-26  1:52       ` Pedro Alves
  2010-03-16 20:49       ` Tom Tromey
  0 siblings, 2 replies; 8+ messages in thread
From: Pedro Alves @ 2010-02-25 19:49 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Thursday 25 February 2010 19:19:28, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Tom> call_function_by_hand installs a momentary breakpoint on std::terminate,
> Tom> and then deletes it later.  However, this can cause a double deletion of
> Tom> the breakpoint.  In the bug, the called function is dlopen, which causes
> Tom> gdb to enter solib_add, which calls breakpoint_re_set, deleting the
> Tom> momentary breakpoint.
> 
> Pedro> Why doesn't stepping over "dlopen", with "(gdb) next" cause the
> Pedro> same problem with step-resume breakpoints?
> 
> The step-resume breakpoints have type bp_step_resume, which are handled
> specially by breakpoint_re_set_one.  The std::terminate breakpoint is
> just a plain bp_breakpoint.

Yes, that's the problem.

> I suppose I could add a new bptype, though that seems rather heavy for
> this.

No momentary breakpoint should have bp_breakpoint type.  All
that do will have the same problem.

I see infcmd.c:finish_backward is also broken in the same way
although finish_forward uses bp_finish.  In fact, it's exactly
this abuse of bp_breakpoint that let to the need of the
make_breakpoint_silent call there...

A `gdb_assert (type != bp_breakpoint)' in
set_momentary_breakpoint would be a Nice To Have.

There's also the option of making the breakpoint at
std::terminate be a real internal breakpoint, enabled/disabled
on need, a-la-E.g., enable_overlay_breakpoints or
set_longjmp_breakpoint.  The advantage is that if the
dlopen causes the first load of libstdc++ ever in
the binary (quite rare, but possible), the breakpoint
at std::terminate resolves itself magically and you
still catch a call to it.  This option requires adding a
breakpoint type as well.

> If you have a particular approach you think would be best, I would be
> happy to implement that.

Thanks.  Take your pick from the above.  I also though
of adding a flag to the breakpoint struct to tag it as
momentary/temporary, but it feels hackish and a bit
lazy given what I said above.  :-)  But I wouldn't say
no to such fix.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-02-25 19:49     ` Pedro Alves
@ 2010-02-26  1:52       ` Pedro Alves
  2010-03-16 20:49       ` Tom Tromey
  1 sibling, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2010-02-26  1:52 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Also see a couple of PRs I just opened related to this.

A real issue/bug, PR11328
<http://sourceware.org/bugzilla/show_bug.cgi?id=11328>

 GDB can miss catching std::terminate calls.  Making
 breakpoint resetting not wipe the std::terminate breakpoint
 should fix this as well.  Your patch wouldn't fix this.

Cosmetic issue, PR11327:

 <http://sourceware.org/bugzilla/show_bug.cgi?id=11327>

 With your patch, gdb would show "Breakpoint -5", or some other
 negative number instead.  This must have been the problem that
 let to the make_breakpoint_silen workaround I mentioned before.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-02-25 19:49     ` Pedro Alves
  2010-02-26  1:52       ` Pedro Alves
@ 2010-03-16 20:49       ` Tom Tromey
  2010-03-25 18:14         ` Pedro Alves
  1 sibling, 1 reply; 8+ messages in thread
From: Tom Tromey @ 2010-03-16 20:49 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro> A `gdb_assert (type != bp_breakpoint)' in
Pedro> set_momentary_breakpoint would be a Nice To Have.

I didn't do this, as I'm not familiar enough with the process record
code.

Pedro> There's also the option of making the breakpoint at
Pedro> std::terminate be a real internal breakpoint, enabled/disabled
Pedro> on need, a-la-E.g., enable_overlay_breakpoints or
Pedro> set_longjmp_breakpoint.

I took this route.

Pedro> Also see a couple of PRs I just opened related to this.
Pedro> <http://sourceware.org/bugzilla/show_bug.cgi?id=11328>
Pedro>  <http://sourceware.org/bugzilla/show_bug.cgi?id=11327>

This patch fixes these problems as well.  Thanks for pointing them out.

There are a couple of things in this patch that may be unusual.

First, I changed the type of stop_stack_dummy to be an enum.  Then I use
this to record whether we hit a stack dummy or a std::terminate
breakpoint.  This seemed pretty direct and ok to me, but if you want
something else, just say what and I will implement it.  I think the
names of the enumerators are not super.

Second, create_std_terminate_master_breakpoint had to use
lookup_minimal_symbol and not lookup_minimal_symbol_text, because we
pass in the demangled name.  I could change that and use _ZSt9terminatev,
but that seemed to be a bit too chummy with the C++ ABI.

Built and regtested on x86-64 (compile farm).  I also ran my valgrind
test case by hand.

Tom

2010-03-16  Tom Tromey  <tromey@redhat.com>

	PR gdb/11327, PR gdb/11328:
	* infrun.c (handle_inferior_event): Change initialization of
	stop_stack_dummy.
	(handle_inferior_event): Change assignment to stop_stack_dummy.
	(normal_stop): Update use of stop_stack_dummy.
	(struct inferior_status) <stop_stack_dummy>: Change type.
	* inferior.h (stop_stack_dummy): Update.
	* infcmd.c (stop_stack_dummy): Change type.
	* infcall.c (cleanup_delete_std_terminate_breakpoint): New
	function.
	(call_function_by_hand): Call set_std_terminate_breakpoint.
	Rewrite std::terminate handling.
	* breakpoint.h (enum bptype) <bp_std_terminate,
	bp_std_terminate_master>: New.
	(enum stop_stack_kind): New.
	(struct bpstat_what) <call_dummy>: Change type.
	(set_std_terminate_breakpoint, delete_std_terminate_breakpoint):
	Declare.
	* breakpoint.c (create_std_terminate_master_breakpoint): New
	function.
	(update_breakpoints_after_exec): Handle bp_std_terminate_master.
	Call create_std_terminate_master_breakpoint.
	(breakpoint_init_inferior): Handle bp_std_terminate.
	(print_it_typical): Handle new breakpoint kinds.
	(bpstat_stop_status): Handle bp_std_terminate_master.
	(bpstat_what): Correctly set call_dummy field.  Handle
	bp_std_terminate_master and bp_std_terminate.
	(print_one_breakpoint_location): Update.
	(allocate_bp_location): Update.
	(set_std_terminate_breakpoint): New function.
	(delete_std_terminate_breakpoint): Likewise.
	(create_thread_event_breakpoint): Update.
	(delete_command): Update.
	(breakpoint_re_set_one): Update.
	(breakpoint_re_set): Call create_std_terminate_master_breakpoint.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 628f2f7..df6775f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1951,6 +1951,41 @@ create_longjmp_master_breakpoint (char *func_name)
   do_cleanups (old_chain);
 }
 
+/* Create a master std::terminate breakpoint.  The actual function
+   looked for is named FUNC_NAME.  */
+static void
+create_std_terminate_master_breakpoint (const char *func_name)
+{
+  struct program_space *pspace;
+  struct objfile *objfile;
+  struct cleanup *old_chain;
+
+  old_chain = save_current_program_space ();
+
+  ALL_PSPACES (pspace)
+    ALL_OBJFILES (objfile)
+    {
+      struct breakpoint *b;
+      struct minimal_symbol *m;
+
+      set_current_program_space (pspace);
+
+      m = lookup_minimal_symbol (func_name, NULL, objfile);
+      if (m == NULL || (MSYMBOL_TYPE (m) != mst_text
+			&& MSYMBOL_TYPE (m) != mst_file_text))
+        continue;
+
+      b = create_internal_breakpoint (get_objfile_arch (objfile),
+				      SYMBOL_VALUE_ADDRESS (m),
+                                      bp_std_terminate_master);
+      b->addr_string = xstrdup (func_name);
+      b->enable_state = bp_disabled;
+    }
+  update_global_location_list (1);
+
+  do_cleanups (old_chain);
+}
+
 void
 update_breakpoints_after_exec (void)
 {
@@ -1992,7 +2027,7 @@ update_breakpoints_after_exec (void)
     /* Thread event breakpoints must be set anew after an exec(),
        as must overlay event and longjmp master breakpoints.  */
     if (b->type == bp_thread_event || b->type == bp_overlay_event
-	|| b->type == bp_longjmp_master)
+	|| b->type == bp_longjmp_master || b->type == bp_std_terminate_master)
       {
 	delete_breakpoint (b);
 	continue;
@@ -2068,6 +2103,7 @@ update_breakpoints_after_exec (void)
   create_longjmp_master_breakpoint ("_longjmp");
   create_longjmp_master_breakpoint ("siglongjmp");
   create_longjmp_master_breakpoint ("_siglongjmp");
+  create_std_terminate_master_breakpoint ("std::terminate()");
 }
 
 int
@@ -2295,6 +2331,7 @@ breakpoint_init_inferior (enum inf_context context)
     switch (b->type)
       {
       case bp_call_dummy:
+      case bp_std_terminate:
 
 	/* If the call dummy breakpoint is at the entry point it will
 	   cause problems when the inferior is rerun, so we better get
@@ -2993,6 +3030,12 @@ print_it_typical (bpstat bs)
       result = PRINT_NOTHING;
       break;
 
+    case bp_std_terminate_master:
+      /* These should never be enabled.  */
+      printf_filtered (_("std::terminate Master Breakpoint: gdb should not stop!\n"));
+      result = PRINT_NOTHING;
+      break;
+
     case bp_watchpoint:
     case bp_hardware_watchpoint:
       annotate_watchpoint (b->number);
@@ -3083,6 +3126,7 @@ print_it_typical (bpstat bs)
     case bp_step_resume:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_tracepoint:
     case bp_fast_tracepoint:
     case bp_jit_event:
@@ -3825,7 +3869,8 @@ bpstat_stop_status (struct address_space *aspace,
 	    continue;
 
 	  if (b->type == bp_thread_event || b->type == bp_overlay_event
-	      || b->type == bp_longjmp_master)
+	      || b->type == bp_longjmp_master
+	      || b->type == bp_std_terminate_master)
 	    /* We do not stop for these.  */
 	    bs->stop = 0;
 	  else
@@ -4033,7 +4078,7 @@ bpstat_what (bpstat bs)
   enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
   struct bpstat_what retval;
 
-  retval.call_dummy = 0;
+  retval.call_dummy = STOP_NONE;
   for (; bs != NULL; bs = bs->next)
     {
       enum class bs_class = no_effect;
@@ -4106,6 +4151,7 @@ bpstat_what (bpstat bs)
 	case bp_thread_event:
 	case bp_overlay_event:
 	case bp_longjmp_master:
+	case bp_std_terminate_master:
 	  bs_class = bp_nostop;
 	  break;
 	case bp_catchpoint:
@@ -4125,7 +4171,13 @@ bpstat_what (bpstat bs)
 	  /* Make sure the action is stop (silent or noisy),
 	     so infrun.c pops the dummy frame.  */
 	  bs_class = bp_silent;
-	  retval.call_dummy = 1;
+	  retval.call_dummy = STOP_STACK_DUMMY;
+	  break;
+	case bp_std_terminate:
+	  /* Make sure the action is stop (silent or noisy),
+	     so infrun.c pops the dummy frame.  */
+	  bs_class = bp_silent;
+	  retval.call_dummy = STOP_STD_TERMINATE;
 	  break;
 	case bp_tracepoint:
 	case bp_fast_tracepoint:
@@ -4253,10 +4305,12 @@ print_one_breakpoint_location (struct breakpoint *b,
     {bp_step_resume, "step resume"},
     {bp_watchpoint_scope, "watchpoint scope"},
     {bp_call_dummy, "call dummy"},
+    {bp_std_terminate, "std::terminate"},
     {bp_shlib_event, "shlib events"},
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
     {bp_longjmp_master, "longjmp master"},
+    {bp_std_terminate_master, "std::terminate master"},
     {bp_catchpoint, "catchpoint"},
     {bp_tracepoint, "tracepoint"},
     {bp_fast_tracepoint, "fast tracepoint"},
@@ -4384,10 +4438,12 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_step_resume:
       case bp_watchpoint_scope:
       case bp_call_dummy:
+      case bp_std_terminate:
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
       case bp_longjmp_master:
+      case bp_std_terminate_master:
       case bp_tracepoint:
       case bp_fast_tracepoint:
       case bp_jit_event:
@@ -5043,11 +5099,13 @@ allocate_bp_location (struct breakpoint *bpt)
     case bp_step_resume:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_shlib_event:
     case bp_thread_event:
     case bp_overlay_event:
     case bp_jit_event:
     case bp_longjmp_master:
+    case bp_std_terminate_master:
       loc->loc_type = bp_loc_software_breakpoint;
       break;
     case bp_hardware_breakpoint:
@@ -5300,6 +5358,33 @@ disable_overlay_breakpoints (void)
     }
 }
 
+/* Set an active std::terminate breakpoint for each std::terminate
+   master breakpoint.  */
+void
+set_std_terminate_breakpoint (void)
+{
+  struct breakpoint *b, *temp;
+
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->pspace == current_program_space
+	&& b->type == bp_std_terminate_master)
+      {
+	struct breakpoint *clone = clone_momentary_breakpoint (b);
+	clone->type = bp_std_terminate;
+      }
+}
+
+/* Delete all the std::terminate breakpoints.  */
+void
+delete_std_terminate_breakpoint (void)
+{
+  struct breakpoint *b, *temp;
+
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_std_terminate)
+      delete_breakpoint (b);
+}
+
 struct breakpoint *
 create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 {
@@ -6342,12 +6427,14 @@ mention (struct breakpoint *b)
       case bp_longjmp_resume:
       case bp_step_resume:
       case bp_call_dummy:
+      case bp_std_terminate:
       case bp_watchpoint_scope:
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
       case bp_jit_event:
       case bp_longjmp_master:
+      case bp_std_terminate_master:
 	break;
       }
 
@@ -9026,11 +9113,13 @@ delete_command (char *arg, int from_tty)
       ALL_BREAKPOINTS (b)
       {
 	if (b->type != bp_call_dummy
+	    && b->type != bp_std_terminate
 	    && b->type != bp_shlib_event
 	    && b->type != bp_jit_event
 	    && b->type != bp_thread_event
 	    && b->type != bp_overlay_event
 	    && b->type != bp_longjmp_master
+	    && b->type != bp_std_terminate_master
 	    && b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -9045,11 +9134,13 @@ delete_command (char *arg, int from_tty)
 	  ALL_BREAKPOINTS_SAFE (b, temp)
 	  {
 	    if (b->type != bp_call_dummy
+		&& b->type != bp_std_terminate
 		&& b->type != bp_shlib_event
 		&& b->type != bp_thread_event
 		&& b->type != bp_jit_event
 		&& b->type != bp_overlay_event
 		&& b->type != bp_longjmp_master
+		&& b->type != bp_std_terminate_master
 		&& b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -9360,6 +9451,7 @@ breakpoint_re_set_one (void *bint)
 	 reset later by breakpoint_re_set.  */
     case bp_overlay_event:
     case bp_longjmp_master:
+    case bp_std_terminate_master:
       delete_breakpoint (b);
       break;
 
@@ -9379,6 +9471,7 @@ breakpoint_re_set_one (void *bint)
     case bp_finish:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_step_resume:
     case bp_longjmp:
     case bp_longjmp_resume:
@@ -9424,6 +9517,7 @@ breakpoint_re_set (void)
   create_longjmp_master_breakpoint ("_longjmp");
   create_longjmp_master_breakpoint ("siglongjmp");
   create_longjmp_master_breakpoint ("_siglongjmp");
+  create_std_terminate_master_breakpoint ("std::terminate()");
 }
 \f
 /* Reset the thread number of this breakpoint:
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 73e2223..3c1db4b 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -83,6 +83,10 @@ enum bptype
        of scope (with hardware support for watchpoints)).  */
     bp_call_dummy,
 
+    /* A breakpoint set on std::terminate, that is used to catch
+       otherwise uncaught exceptions thrown during an inferior call.  */
+    bp_std_terminate,
+
     /* Some dynamic linkers (HP, maybe Solaris) can arrange for special
        code in the inferior to run when significant events occur in the
        dynamic linker (for example a library is loaded or unloaded).
@@ -118,6 +122,9 @@ enum bptype
 
     bp_longjmp_master,
 
+    /* Master copies of std::terminate breakpoints.  */
+    bp_std_terminate_master,
+
     bp_catchpoint,
 
     bp_tracepoint,
@@ -601,6 +608,20 @@ enum bpstat_what_main_action
     BPSTAT_WHAT_LAST
   };
 
+/* An enum indicating the kind of "stack dummy" stop.  This is a bit
+   of a misnomer because only one kind of truly a stack dummy.  */
+enum stop_stack_kind
+  {
+    /* We didn't stop at a stack dummy breakpoint.  */
+    STOP_NONE = 0,
+
+    /* Stopped at a stack dummy.  */
+    STOP_STACK_DUMMY,
+
+    /* Stopped at std::terminate.  */
+    STOP_STD_TERMINATE
+  };
+
 struct bpstat_what
   {
     enum bpstat_what_main_action main_action;
@@ -609,7 +630,7 @@ struct bpstat_what
        of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
        continuing from a call dummy without popping the frame is not a
        useful one).  */
-    int call_dummy;
+    enum stop_stack_kind call_dummy;
   };
 
 /* The possible return values for print_bpstat, print_it_normal,
@@ -851,6 +872,9 @@ extern void delete_longjmp_breakpoint (int thread);
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
+extern void set_std_terminate_breakpoint (void);
+extern void delete_std_terminate_breakpoint (void);
+
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 2d8bd2c..537e0b6 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -402,6 +402,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
   return e;
 }
 
+/* A cleanup function that calls delete_std_terminate_breakpoint.  */
+static void
+cleanup_delete_std_terminate_breakpoint (void *ignore)
+{
+  delete_std_terminate_breakpoint ();
+}
+
 /* All this stuff with a dummy frame may seem unnecessarily complicated
    (why not just save registers in GDB?).  The purpose of pushing a dummy
    frame which looks just like a real frame is so that if you call a
@@ -439,9 +446,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   struct cleanup *args_cleanup;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
-  struct breakpoint *terminate_bp = NULL;
-  struct minimal_symbol *tm;
-  struct cleanup *terminate_bp_cleanup = NULL;
+  struct cleanup *terminate_bp_cleanup;
   ptid_t call_thread_ptid;
   struct gdb_exception e;
   const char *name;
@@ -753,13 +758,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
      call.  Place a momentary breakpoint in the std::terminate function
      and if triggered in the call, rewind.  */
   if (unwind_on_terminating_exception_p)
-     {
-       struct minimal_symbol *tm = lookup_minimal_symbol  ("std::terminate()",
-							   NULL, NULL);
-       if (tm != NULL)
-	   terminate_bp = set_momentary_breakpoint_at_pc
-	     (gdbarch, SYMBOL_VALUE_ADDRESS (tm),  bp_breakpoint);
-     }
+    set_std_terminate_breakpoint ();
 
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
@@ -772,8 +771,8 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   discard_cleanups (inf_status_cleanup);
 
   /* Register a clean-up for unwind_on_terminating_exception_breakpoint.  */
-  if (terminate_bp)
-    terminate_bp_cleanup = make_cleanup_delete_breakpoint (terminate_bp);
+  terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
+				       NULL);
 
   /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
      If you're looking to implement asynchronous dummy-frames, then
@@ -874,7 +873,7 @@ When the function is done executing, GDB will silently stop."),
 	       name);
     }
 
-  if (stopped_by_random_signal || !stop_stack_dummy)
+  if (stopped_by_random_signal || stop_stack_dummy != STOP_STACK_DUMMY)
     {
       const char *name = get_function_name (funaddr,
 					    name_buf, sizeof (name_buf));
@@ -928,30 +927,17 @@ When the function is done executing, GDB will silently stop."),
 	    }
 	}
 
-      if (!stop_stack_dummy)
+      if (stop_stack_dummy == STOP_STD_TERMINATE)
 	{
+	  /* We must get back to the frame we were before the dummy
+	     call.  */
+	  dummy_frame_pop (dummy_id);
 
-	  /* Check if unwind on terminating exception behaviour is on.  */
-	  if (unwind_on_terminating_exception_p)
-	    {
-	      /* Check that the breakpoint is our special std::terminate
-		 breakpoint.  If it is, we do not want to kill the inferior
-		 in an inferior function call. Rewind, and warn the
-		 user.  */
-
-	      if (terminate_bp != NULL
-		  && (inferior_thread ()->stop_bpstat->breakpoint_at->address
-		      == terminate_bp->loc->address))
-		{
-		  /* We must get back to the frame we were before the
-		     dummy call.  */
-		  dummy_frame_pop (dummy_id);
-
-		  /* We also need to restore inferior status to that before the
-		     dummy call.  */
-		  restore_inferior_status (inf_status);
-
-		  error (_("\
+	  /* We also need to restore inferior status to that before
+	     the dummy call.  */
+	  restore_inferior_status (inf_status);
+
+	  error (_("\
 The program being debugged entered a std::terminate call, most likely\n\
 caused by an unhandled C++ exception.  GDB blocked this call in order\n\
 to prevent the program from being terminated, and has restored the\n\
@@ -959,9 +945,11 @@ context to its original state before the call.\n\
 To change this behaviour use \"set unwind-on-terminating-exception off\".\n\
 Evaluation of the expression containing the function (%s)\n\
 will be abandoned."),
-			 name);
-		}
-	    }
+		 name);
+	}
+      else if (stop_stack_dummy == STOP_NONE)
+	{
+
 	  /* We hit a breakpoint inside the FUNCTION.
 	     Keep the dummy frame, the user may want to examine its state.
 	     Discard inferior status, we're not at the same point
@@ -988,10 +976,7 @@ When the function is done executing, GDB will silently stop."),
       internal_error (__FILE__, __LINE__, _("... should not be here"));
     }
 
-  /* If we get here and the std::terminate() breakpoint has been set,
-     it has to be cleaned manually.  */
-  if (terminate_bp)
-    do_cleanups (terminate_bp_cleanup);
+  do_cleanups (terminate_bp_cleanup);
 
   /* If we get here the called FUNCTION ran to completion,
      and the dummy frame has already been popped.  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 29cf427..12382bc 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -156,7 +156,7 @@ int breakpoint_proceeded;
 
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
-int stop_stack_dummy;
+enum stop_stack_kind stop_stack_dummy;
 
 /* Nonzero if stopped due to a random (unexpected) signal in inferior
    process.  */
diff --git a/gdb/inferior.h b/gdb/inferior.h
index dc87a9e..17b2c6f 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -297,7 +297,7 @@ extern CORE_ADDR stop_pc;
 
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
-extern int stop_stack_dummy;
+extern enum stop_stack_kind stop_stack_dummy;
 
 /* Nonzero if program stopped due to a random (unexpected) signal in
    inferior process.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 83cfe3c..f5369cd 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2904,7 +2904,7 @@ handle_inferior_event (struct execution_control_state *ecs)
   target_last_waitstatus = ecs->ws;
 
   /* Always clear state belonging to the previous time we stopped.  */
-  stop_stack_dummy = 0;
+  stop_stack_dummy = STOP_NONE;
 
   /* If it's a new process, add it to the thread database */
 
@@ -3970,7 +3970,7 @@ process_event_stop_test:
 
     if (what.call_dummy)
       {
-	stop_stack_dummy = 1;
+	stop_stack_dummy = what.call_dummy;
       }
 
     switch (what.main_action)
@@ -5460,7 +5460,7 @@ Further execution is probably impossible.\n"));
       stop_registers = regcache_dup (get_current_regcache ());
     }
 
-  if (stop_stack_dummy)
+  if (stop_stack_dummy == STOP_STACK_DUMMY)
     {
       /* Pop the empty frame that contains the stack dummy.
 	 This also restores inferior state prior to the call
@@ -6038,7 +6038,7 @@ struct inferior_status
 {
   bpstat stop_bpstat;
   int stop_step;
-  int stop_stack_dummy;
+  enum stop_stack_kind stop_stack_dummy;
   int stopped_by_random_signal;
   int stepping_over_breakpoint;
   CORE_ADDR step_range_start;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-03-16 20:49       ` Tom Tromey
@ 2010-03-25 18:14         ` Pedro Alves
  2010-03-25 20:37           ` Tom Tromey
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2010-03-25 18:14 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Hi Tom, sorry for the delay.

On Tuesday 16 March 2010 20:49:03, Tom Tromey wrote:
> Pedro> A `gdb_assert (type != bp_breakpoint)' in
> Pedro> set_momentary_breakpoint would be a Nice To Have.
> 
> I didn't do this, as I'm not familiar enough with the process record
> code.

That's fine.

> 
> Pedro> There's also the option of making the breakpoint at
> Pedro> std::terminate be a real internal breakpoint, enabled/disabled
> Pedro> on need, a-la-E.g., enable_overlay_breakpoints or
> Pedro> set_longjmp_breakpoint.
> 
> I took this route.

Thanks.

> 
> Pedro> Also see a couple of PRs I just opened related to this.
> Pedro> <http://sourceware.org/bugzilla/show_bug.cgi?id=11328>
> Pedro>  <http://sourceware.org/bugzilla/show_bug.cgi?id=11327>
> 
> This patch fixes these problems as well.  Thanks for pointing them out.

Great!

> First, I changed the type of stop_stack_dummy to be an enum.  Then I use
> this to record whether we hit a stack dummy or a std::terminate
> breakpoint.  This seemed pretty direct and ok to me, but if you want
> something else, just say what and I will implement it.  I think the
> names of the enumerators are not super.

Yeah.  I'm fine with leaving as is.  Dunno what others think.
I guess we could get rid of this variable altogether by looking
over the stop_bpstat of the thread instead.

> 2010-03-16  Tom Tromey  <tromey@redhat.com>
> 
>         PR gdb/11327, PR gdb/11328:

PR11368 as well?

>         * infrun.c (handle_inferior_event): Change initialization of
>         stop_stack_dummy.
>         (handle_inferior_event): Change assignment to stop_stack_dummy.
>         (normal_stop): Update use of stop_stack_dummy.
>         (struct inferior_status) <stop_stack_dummy>: Change type.
>         * inferior.h (stop_stack_dummy): Update.
>         * infcmd.c (stop_stack_dummy): Change type.
>         * infcall.c (cleanup_delete_std_terminate_breakpoint): New
>         function.
>         (call_function_by_hand): Call set_std_terminate_breakpoint.
>         Rewrite std::terminate handling.
>         * breakpoint.h (enum bptype) <bp_std_terminate,
>         bp_std_terminate_master>: New.
>         (enum stop_stack_kind): New.
>         (struct bpstat_what) <call_dummy>: Change type.
>         (set_std_terminate_breakpoint, delete_std_terminate_breakpoint):
>         Declare.
>         * breakpoint.c (create_std_terminate_master_breakpoint): New
>         function.
>         (update_breakpoints_after_exec): Handle bp_std_terminate_master.
>         Call create_std_terminate_master_breakpoint.
>         (breakpoint_init_inferior): Handle bp_std_terminate.
>         (print_it_typical): Handle new breakpoint kinds.
>         (bpstat_stop_status): Handle bp_std_terminate_master.
>         (bpstat_what): Correctly set call_dummy field.  Handle
>         bp_std_terminate_master and bp_std_terminate.
>         (print_one_breakpoint_location): Update.
>         (allocate_bp_location): Update.
>         (set_std_terminate_breakpoint): New function.
>         (delete_std_terminate_breakpoint): Likewise.
>         (create_thread_event_breakpoint): Update.
>         (delete_command): Update.
>         (breakpoint_re_set_one): Update.
>         (breakpoint_re_set): Call create_std_terminate_master_breakpoint.

> @@ -2295,6 +2331,7 @@ breakpoint_init_inferior (enum inf_context context)
>      switch (b->type)
>        {
>        case bp_call_dummy:
> +      case bp_std_terminate:


Hmm, why's this needed?  Doesn't the cleanup handling in infcall
always delete these?  longjmp breakpoints also aren't deleted here.
In any case, if this stays for some reason, the comment below
doesn't apply to this breakpoint, so move the case further below,
just before delete_breakpoint, for instance --- notice how each
case has a describing comment below it.

> -      if (!stop_stack_dummy)
> +      if (stop_stack_dummy == STOP_STD_TERMINATE)
>         {
> +         /* We must get back to the frame we were before the dummy
> +            call.  */
> +         dummy_frame_pop (dummy_id);

One thing I wonder is what should happen if there's
a user breakpoint on "std::terminate".  My intuition says that
we should not auto-unwind in that case (user breakpoints should
have higher precedence than internal handlings), but your change
matches the current behavior, so it's fine.

Otherwise, this looks good to me.  Thanks!

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: RFC: fix bug with std::terminate handler
  2010-03-25 18:14         ` Pedro Alves
@ 2010-03-25 20:37           ` Tom Tromey
  0 siblings, 0 replies; 8+ messages in thread
From: Tom Tromey @ 2010-03-25 20:37 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

>> PR gdb/11327, PR gdb/11328:

Pedro> PR11368 as well?

Yeah, thanks.

>> @@ -2295,6 +2331,7 @@ breakpoint_init_inferior (enum inf_context context)
>> switch (b->type)
>> {
>> case bp_call_dummy:
>> +      case bp_std_terminate:

Pedro> Hmm, why's this needed?  Doesn't the cleanup handling in infcall
Pedro> always delete these?

Yes, you are correct.  I removed this.

Pedro> One thing I wonder is what should happen if there's
Pedro> a user breakpoint on "std::terminate".  My intuition says that
Pedro> we should not auto-unwind in that case (user breakpoints should
Pedro> have higher precedence than internal handlings), but your change
Pedro> matches the current behavior, so it's fine.

Thanks.

Pedro> Otherwise, this looks good to me.  Thanks!

I am committing the appended, which is the patch with the above minor
change.

Tom

2010-03-25  Tom Tromey  <tromey@redhat.com>

	PR gdb/11327, PR gdb/11328, PR breakpoints/11368:
	* infrun.c (handle_inferior_event): Change initialization of
	stop_stack_dummy.
	(handle_inferior_event): Change assignment to stop_stack_dummy.
	(normal_stop): Update use of stop_stack_dummy.
	(struct inferior_status) <stop_stack_dummy>: Change type.
	* inferior.h (stop_stack_dummy): Update.
	* infcmd.c (stop_stack_dummy): Change type.
	* infcall.c (cleanup_delete_std_terminate_breakpoint): New
	function.
	(call_function_by_hand): Call set_std_terminate_breakpoint.
	Rewrite std::terminate handling.
	* breakpoint.h (enum bptype) <bp_std_terminate,
	bp_std_terminate_master>: New.
	(enum stop_stack_kind): New.
	(struct bpstat_what) <call_dummy>: Change type.
	(set_std_terminate_breakpoint, delete_std_terminate_breakpoint):
	Declare.
	* breakpoint.c (create_std_terminate_master_breakpoint): New
	function.
	(update_breakpoints_after_exec): Handle bp_std_terminate_master.
	Call create_std_terminate_master_breakpoint.
	(print_it_typical): Handle new breakpoint kinds.
	(bpstat_stop_status): Handle bp_std_terminate_master.
	(bpstat_what): Correctly set call_dummy field.  Handle
	bp_std_terminate_master and bp_std_terminate.
	(print_one_breakpoint_location): Update.
	(allocate_bp_location): Update.
	(set_std_terminate_breakpoint): New function.
	(delete_std_terminate_breakpoint): Likewise.
	(create_thread_event_breakpoint): Update.
	(delete_command): Update.
	(breakpoint_re_set_one): Update.
	(breakpoint_re_set): Call create_std_terminate_master_breakpoint.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f47d63c..981776f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2048,6 +2048,41 @@ create_longjmp_master_breakpoint (char *func_name)
   do_cleanups (old_chain);
 }
 
+/* Create a master std::terminate breakpoint.  The actual function
+   looked for is named FUNC_NAME.  */
+static void
+create_std_terminate_master_breakpoint (const char *func_name)
+{
+  struct program_space *pspace;
+  struct objfile *objfile;
+  struct cleanup *old_chain;
+
+  old_chain = save_current_program_space ();
+
+  ALL_PSPACES (pspace)
+    ALL_OBJFILES (objfile)
+    {
+      struct breakpoint *b;
+      struct minimal_symbol *m;
+
+      set_current_program_space (pspace);
+
+      m = lookup_minimal_symbol (func_name, NULL, objfile);
+      if (m == NULL || (MSYMBOL_TYPE (m) != mst_text
+			&& MSYMBOL_TYPE (m) != mst_file_text))
+        continue;
+
+      b = create_internal_breakpoint (get_objfile_arch (objfile),
+				      SYMBOL_VALUE_ADDRESS (m),
+                                      bp_std_terminate_master);
+      b->addr_string = xstrdup (func_name);
+      b->enable_state = bp_disabled;
+    }
+  update_global_location_list (1);
+
+  do_cleanups (old_chain);
+}
+
 void
 update_breakpoints_after_exec (void)
 {
@@ -2089,7 +2124,7 @@ update_breakpoints_after_exec (void)
     /* Thread event breakpoints must be set anew after an exec(),
        as must overlay event and longjmp master breakpoints.  */
     if (b->type == bp_thread_event || b->type == bp_overlay_event
-	|| b->type == bp_longjmp_master)
+	|| b->type == bp_longjmp_master || b->type == bp_std_terminate_master)
       {
 	delete_breakpoint (b);
 	continue;
@@ -2165,6 +2200,7 @@ update_breakpoints_after_exec (void)
   create_longjmp_master_breakpoint ("_longjmp");
   create_longjmp_master_breakpoint ("siglongjmp");
   create_longjmp_master_breakpoint ("_siglongjmp");
+  create_std_terminate_master_breakpoint ("std::terminate()");
 }
 
 int
@@ -3090,6 +3126,12 @@ print_it_typical (bpstat bs)
       result = PRINT_NOTHING;
       break;
 
+    case bp_std_terminate_master:
+      /* These should never be enabled.  */
+      printf_filtered (_("std::terminate Master Breakpoint: gdb should not stop!\n"));
+      result = PRINT_NOTHING;
+      break;
+
     case bp_watchpoint:
     case bp_hardware_watchpoint:
       annotate_watchpoint (b->number);
@@ -3180,6 +3222,7 @@ print_it_typical (bpstat bs)
     case bp_step_resume:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_tracepoint:
     case bp_fast_tracepoint:
     case bp_jit_event:
@@ -3922,7 +3965,8 @@ bpstat_stop_status (struct address_space *aspace,
 	    continue;
 
 	  if (b->type == bp_thread_event || b->type == bp_overlay_event
-	      || b->type == bp_longjmp_master)
+	      || b->type == bp_longjmp_master
+	      || b->type == bp_std_terminate_master)
 	    /* We do not stop for these.  */
 	    bs->stop = 0;
 	  else
@@ -4130,7 +4174,7 @@ bpstat_what (bpstat bs)
   enum bpstat_what_main_action current_action = BPSTAT_WHAT_KEEP_CHECKING;
   struct bpstat_what retval;
 
-  retval.call_dummy = 0;
+  retval.call_dummy = STOP_NONE;
   for (; bs != NULL; bs = bs->next)
     {
       enum class bs_class = no_effect;
@@ -4203,6 +4247,7 @@ bpstat_what (bpstat bs)
 	case bp_thread_event:
 	case bp_overlay_event:
 	case bp_longjmp_master:
+	case bp_std_terminate_master:
 	  bs_class = bp_nostop;
 	  break;
 	case bp_catchpoint:
@@ -4222,7 +4267,13 @@ bpstat_what (bpstat bs)
 	  /* Make sure the action is stop (silent or noisy),
 	     so infrun.c pops the dummy frame.  */
 	  bs_class = bp_silent;
-	  retval.call_dummy = 1;
+	  retval.call_dummy = STOP_STACK_DUMMY;
+	  break;
+	case bp_std_terminate:
+	  /* Make sure the action is stop (silent or noisy),
+	     so infrun.c pops the dummy frame.  */
+	  bs_class = bp_silent;
+	  retval.call_dummy = STOP_STD_TERMINATE;
 	  break;
 	case bp_tracepoint:
 	case bp_fast_tracepoint:
@@ -4350,10 +4401,12 @@ print_one_breakpoint_location (struct breakpoint *b,
     {bp_step_resume, "step resume"},
     {bp_watchpoint_scope, "watchpoint scope"},
     {bp_call_dummy, "call dummy"},
+    {bp_std_terminate, "std::terminate"},
     {bp_shlib_event, "shlib events"},
     {bp_thread_event, "thread events"},
     {bp_overlay_event, "overlay events"},
     {bp_longjmp_master, "longjmp master"},
+    {bp_std_terminate_master, "std::terminate master"},
     {bp_catchpoint, "catchpoint"},
     {bp_tracepoint, "tracepoint"},
     {bp_fast_tracepoint, "fast tracepoint"},
@@ -4481,10 +4534,12 @@ print_one_breakpoint_location (struct breakpoint *b,
       case bp_step_resume:
       case bp_watchpoint_scope:
       case bp_call_dummy:
+      case bp_std_terminate:
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
       case bp_longjmp_master:
+      case bp_std_terminate_master:
       case bp_tracepoint:
       case bp_fast_tracepoint:
       case bp_jit_event:
@@ -5120,11 +5175,13 @@ allocate_bp_location (struct breakpoint *bpt)
     case bp_step_resume:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_shlib_event:
     case bp_thread_event:
     case bp_overlay_event:
     case bp_jit_event:
     case bp_longjmp_master:
+    case bp_std_terminate_master:
       loc->loc_type = bp_loc_software_breakpoint;
       break;
     case bp_hardware_breakpoint:
@@ -5377,6 +5434,33 @@ disable_overlay_breakpoints (void)
     }
 }
 
+/* Set an active std::terminate breakpoint for each std::terminate
+   master breakpoint.  */
+void
+set_std_terminate_breakpoint (void)
+{
+  struct breakpoint *b, *temp;
+
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->pspace == current_program_space
+	&& b->type == bp_std_terminate_master)
+      {
+	struct breakpoint *clone = clone_momentary_breakpoint (b);
+	clone->type = bp_std_terminate;
+      }
+}
+
+/* Delete all the std::terminate breakpoints.  */
+void
+delete_std_terminate_breakpoint (void)
+{
+  struct breakpoint *b, *temp;
+
+  ALL_BREAKPOINTS_SAFE (b, temp)
+    if (b->type == bp_std_terminate)
+      delete_breakpoint (b);
+}
+
 struct breakpoint *
 create_thread_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
 {
@@ -6422,12 +6506,14 @@ mention (struct breakpoint *b)
       case bp_longjmp_resume:
       case bp_step_resume:
       case bp_call_dummy:
+      case bp_std_terminate:
       case bp_watchpoint_scope:
       case bp_shlib_event:
       case bp_thread_event:
       case bp_overlay_event:
       case bp_jit_event:
       case bp_longjmp_master:
+      case bp_std_terminate_master:
 	break;
       }
 
@@ -9091,11 +9177,13 @@ delete_command (char *arg, int from_tty)
       ALL_BREAKPOINTS (b)
       {
 	if (b->type != bp_call_dummy
+	    && b->type != bp_std_terminate
 	    && b->type != bp_shlib_event
 	    && b->type != bp_jit_event
 	    && b->type != bp_thread_event
 	    && b->type != bp_overlay_event
 	    && b->type != bp_longjmp_master
+	    && b->type != bp_std_terminate_master
 	    && b->number >= 0)
 	  {
 	    breaks_to_delete = 1;
@@ -9110,11 +9198,13 @@ delete_command (char *arg, int from_tty)
 	  ALL_BREAKPOINTS_SAFE (b, temp)
 	  {
 	    if (b->type != bp_call_dummy
+		&& b->type != bp_std_terminate
 		&& b->type != bp_shlib_event
 		&& b->type != bp_thread_event
 		&& b->type != bp_jit_event
 		&& b->type != bp_overlay_event
 		&& b->type != bp_longjmp_master
+		&& b->type != bp_std_terminate_master
 		&& b->number >= 0)
 	      delete_breakpoint (b);
 	  }
@@ -9425,6 +9515,7 @@ breakpoint_re_set_one (void *bint)
 	 reset later by breakpoint_re_set.  */
     case bp_overlay_event:
     case bp_longjmp_master:
+    case bp_std_terminate_master:
       delete_breakpoint (b);
       break;
 
@@ -9444,6 +9535,7 @@ breakpoint_re_set_one (void *bint)
     case bp_finish:
     case bp_watchpoint_scope:
     case bp_call_dummy:
+    case bp_std_terminate:
     case bp_step_resume:
     case bp_longjmp:
     case bp_longjmp_resume:
@@ -9489,6 +9581,7 @@ breakpoint_re_set (void)
   create_longjmp_master_breakpoint ("_longjmp");
   create_longjmp_master_breakpoint ("siglongjmp");
   create_longjmp_master_breakpoint ("_siglongjmp");
+  create_std_terminate_master_breakpoint ("std::terminate()");
 }
 \f
 /* Reset the thread number of this breakpoint:
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 3f194c6..22cc133 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -83,6 +83,10 @@ enum bptype
        of scope (with hardware support for watchpoints)).  */
     bp_call_dummy,
 
+    /* A breakpoint set on std::terminate, that is used to catch
+       otherwise uncaught exceptions thrown during an inferior call.  */
+    bp_std_terminate,
+
     /* Some dynamic linkers (HP, maybe Solaris) can arrange for special
        code in the inferior to run when significant events occur in the
        dynamic linker (for example a library is loaded or unloaded).
@@ -118,6 +122,9 @@ enum bptype
 
     bp_longjmp_master,
 
+    /* Master copies of std::terminate breakpoints.  */
+    bp_std_terminate_master,
+
     bp_catchpoint,
 
     bp_tracepoint,
@@ -598,6 +605,20 @@ enum bpstat_what_main_action
     BPSTAT_WHAT_LAST
   };
 
+/* An enum indicating the kind of "stack dummy" stop.  This is a bit
+   of a misnomer because only one kind of truly a stack dummy.  */
+enum stop_stack_kind
+  {
+    /* We didn't stop at a stack dummy breakpoint.  */
+    STOP_NONE = 0,
+
+    /* Stopped at a stack dummy.  */
+    STOP_STACK_DUMMY,
+
+    /* Stopped at std::terminate.  */
+    STOP_STD_TERMINATE
+  };
+
 struct bpstat_what
   {
     enum bpstat_what_main_action main_action;
@@ -606,7 +627,7 @@ struct bpstat_what
        of BPSTAT_WHAT_STOP_SILENT or BPSTAT_WHAT_STOP_NOISY (the concept of
        continuing from a call dummy without popping the frame is not a
        useful one).  */
-    int call_dummy;
+    enum stop_stack_kind call_dummy;
   };
 
 /* The possible return values for print_bpstat, print_it_normal,
@@ -851,6 +872,9 @@ extern void delete_longjmp_breakpoint (int thread);
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
 
+extern void set_std_terminate_breakpoint (void);
+extern void delete_std_terminate_breakpoint (void);
+
 /* These functions respectively disable or reenable all currently
    enabled watchpoints.  When disabled, the watchpoints are marked
    call_disabled.  When reenabled, they are marked enabled.
diff --git a/gdb/infcall.c b/gdb/infcall.c
index d6a8de3..b603cf6 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -403,6 +403,13 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
   return e;
 }
 
+/* A cleanup function that calls delete_std_terminate_breakpoint.  */
+static void
+cleanup_delete_std_terminate_breakpoint (void *ignore)
+{
+  delete_std_terminate_breakpoint ();
+}
+
 /* All this stuff with a dummy frame may seem unnecessarily complicated
    (why not just save registers in GDB?).  The purpose of pushing a dummy
    frame which looks just like a real frame is so that if you call a
@@ -440,9 +447,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   struct cleanup *args_cleanup;
   struct frame_info *frame;
   struct gdbarch *gdbarch;
-  struct breakpoint *terminate_bp = NULL;
-  struct minimal_symbol *tm;
-  struct cleanup *terminate_bp_cleanup = NULL;
+  struct cleanup *terminate_bp_cleanup;
   ptid_t call_thread_ptid;
   struct gdb_exception e;
   const char *name;
@@ -757,13 +762,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
      call.  Place a momentary breakpoint in the std::terminate function
      and if triggered in the call, rewind.  */
   if (unwind_on_terminating_exception_p)
-     {
-       struct minimal_symbol *tm = lookup_minimal_symbol  ("std::terminate()",
-							   NULL, NULL);
-       if (tm != NULL)
-	   terminate_bp = set_momentary_breakpoint_at_pc
-	     (gdbarch, SYMBOL_VALUE_ADDRESS (tm),  bp_breakpoint);
-     }
+    set_std_terminate_breakpoint ();
 
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
@@ -776,8 +775,8 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   discard_cleanups (inf_status_cleanup);
 
   /* Register a clean-up for unwind_on_terminating_exception_breakpoint.  */
-  if (terminate_bp)
-    terminate_bp_cleanup = make_cleanup_delete_breakpoint (terminate_bp);
+  terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
+				       NULL);
 
   /* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
      If you're looking to implement asynchronous dummy-frames, then
@@ -878,7 +877,7 @@ When the function is done executing, GDB will silently stop."),
 	       name);
     }
 
-  if (stopped_by_random_signal || !stop_stack_dummy)
+  if (stopped_by_random_signal || stop_stack_dummy != STOP_STACK_DUMMY)
     {
       const char *name = get_function_name (funaddr,
 					    name_buf, sizeof (name_buf));
@@ -932,30 +931,17 @@ When the function is done executing, GDB will silently stop."),
 	    }
 	}
 
-      if (!stop_stack_dummy)
+      if (stop_stack_dummy == STOP_STD_TERMINATE)
 	{
+	  /* We must get back to the frame we were before the dummy
+	     call.  */
+	  dummy_frame_pop (dummy_id);
 
-	  /* Check if unwind on terminating exception behaviour is on.  */
-	  if (unwind_on_terminating_exception_p)
-	    {
-	      /* Check that the breakpoint is our special std::terminate
-		 breakpoint.  If it is, we do not want to kill the inferior
-		 in an inferior function call. Rewind, and warn the
-		 user.  */
-
-	      if (terminate_bp != NULL
-		  && (inferior_thread ()->stop_bpstat->breakpoint_at->address
-		      == terminate_bp->loc->address))
-		{
-		  /* We must get back to the frame we were before the
-		     dummy call.  */
-		  dummy_frame_pop (dummy_id);
-
-		  /* We also need to restore inferior status to that before the
-		     dummy call.  */
-		  restore_inferior_status (inf_status);
-
-		  error (_("\
+	  /* We also need to restore inferior status to that before
+	     the dummy call.  */
+	  restore_inferior_status (inf_status);
+
+	  error (_("\
 The program being debugged entered a std::terminate call, most likely\n\
 caused by an unhandled C++ exception.  GDB blocked this call in order\n\
 to prevent the program from being terminated, and has restored the\n\
@@ -963,9 +949,11 @@ context to its original state before the call.\n\
 To change this behaviour use \"set unwind-on-terminating-exception off\".\n\
 Evaluation of the expression containing the function (%s)\n\
 will be abandoned."),
-			 name);
-		}
-	    }
+		 name);
+	}
+      else if (stop_stack_dummy == STOP_NONE)
+	{
+
 	  /* We hit a breakpoint inside the FUNCTION.
 	     Keep the dummy frame, the user may want to examine its state.
 	     Discard inferior status, we're not at the same point
@@ -992,10 +980,7 @@ When the function is done executing, GDB will silently stop."),
       internal_error (__FILE__, __LINE__, _("... should not be here"));
     }
 
-  /* If we get here and the std::terminate() breakpoint has been set,
-     it has to be cleaned manually.  */
-  if (terminate_bp)
-    do_cleanups (terminate_bp_cleanup);
+  do_cleanups (terminate_bp_cleanup);
 
   /* If we get here the called FUNCTION ran to completion,
      and the dummy frame has already been popped.  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 174acde..db2232d 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -155,7 +155,7 @@ int breakpoint_proceeded;
 
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
-int stop_stack_dummy;
+enum stop_stack_kind stop_stack_dummy;
 
 /* Nonzero if stopped due to a random (unexpected) signal in inferior
    process.  */
diff --git a/gdb/inferior.h b/gdb/inferior.h
index dc87a9e..17b2c6f 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -297,7 +297,7 @@ extern CORE_ADDR stop_pc;
 
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
-extern int stop_stack_dummy;
+extern enum stop_stack_kind stop_stack_dummy;
 
 /* Nonzero if program stopped due to a random (unexpected) signal in
    inferior process.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4bfe546..5f58759 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2904,7 +2904,7 @@ handle_inferior_event (struct execution_control_state *ecs)
   target_last_waitstatus = ecs->ws;
 
   /* Always clear state belonging to the previous time we stopped.  */
-  stop_stack_dummy = 0;
+  stop_stack_dummy = STOP_NONE;
 
   /* If it's a new process, add it to the thread database */
 
@@ -3970,7 +3970,7 @@ process_event_stop_test:
 
     if (what.call_dummy)
       {
-	stop_stack_dummy = 1;
+	stop_stack_dummy = what.call_dummy;
       }
 
     switch (what.main_action)
@@ -5460,7 +5460,7 @@ Further execution is probably impossible.\n"));
       stop_registers = regcache_dup (get_current_regcache ());
     }
 
-  if (stop_stack_dummy)
+  if (stop_stack_dummy == STOP_STACK_DUMMY)
     {
       /* Pop the empty frame that contains the stack dummy.
 	 This also restores inferior state prior to the call
@@ -6038,7 +6038,7 @@ struct inferior_status
 {
   bpstat stop_bpstat;
   int stop_step;
-  int stop_stack_dummy;
+  enum stop_stack_kind stop_stack_dummy;
   int stopped_by_random_signal;
   int stepping_over_breakpoint;
   CORE_ADDR step_range_start;

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-03-25 20:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-25 18:11 RFC: fix bug with std::terminate handler Tom Tromey
2010-02-25 18:26 ` Pedro Alves
2010-02-25 19:19   ` Tom Tromey
2010-02-25 19:49     ` Pedro Alves
2010-02-26  1:52       ` Pedro Alves
2010-03-16 20:49       ` Tom Tromey
2010-03-25 18:14         ` Pedro Alves
2010-03-25 20:37           ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).