[RFC] Patch for gnats pr 2495

[Phil Muldoon <pmuldoon at redhat dot com>]

This is a draft patch for comments. It addresses the bug detailed in 
gnats pr 2495. A brief summary:

For most systems GCC uses the exception handling strategy documented 
here:  http://www.codesourcery.com/public/cxx-abi/

This strategy tries to find an exception handler firstly "in frame"; if 
none is found, it unwinds the stack until it finds a handler that can 
handle the exception. If it cannot find an exception handler anywhere on 
the stack, it calls std::terminate. This function checks  if there is a 
custom "default exception handler" and if found, executes that. If it 
cannot find a custom "default exception handler", or when the custom 
handler returns, std::abort is called and the process exits after being 
delivered a SIGABRT. This is the "simple" version of events, but that is 
the basic strategy. In the normal execution of a process, this is sane 
and correct -  if there is no handler for an exception anywhere on the 
stack, there is little else than can be done. However in the 
artificially constructed environment of the debugger controlling the 
inferior, it can become problematic in some situations. Inferior 
function calls are one of the situations where this becomes problematic.

In GDB inferior function calls are very common. For example:

(gdb) print fooclass.foo()

Will print the result (return value) of that function. To execute that 
function outside of the normal program flow, GDB must create a dummy 
frame to execute the (inferior) function (call). This isolated and dummy 
frame is problematic with C++ exceptions as it cannot be unwound (moving 
up the stack) beyond the dummy frame,. If an exception is generated by 
foo() when the function is being executed in the dummy frame and it does 
not happen to have an in-frame exception handler, the whole inferior is 
(wrongly) terminated. This is because any out-of-frame exception handler 
that usually handles exceptions in function foo() and are normally "up" 
in the stack will not be considered. The inferior is terminated as 
detailed above, has to be restarted, and brought back to state by the  
user.  This is wrong in my opinion. It is perfectly legal to have a 
function handler out-of-frame, and in normal execution it would be found 
and handled; but the artificial environment constructed around the frame 
for the purpose of the inferior function call "fools" the unwinder and 
terminates the inferior.

This patch addresses that by gating access to std::terminate in an 
inferior function call. If it enters that function, it is popped back to 
the calling state and "rescues" the inferior. This patch also adds the 
functionality to turn this gating behaviour on or off. The default is on.

gdb/ChangeLog
2008-10-13  Phil Muldoon  <pmuldoon@redhat.com>

   * infcall.c (show_unwind_on_terminating_exception_p): New
   function.
   (call_function_by_hand): Create breakpoint and clean-up call for
   std::terminate.breakpoint. Add unwind_on_terminating_exception_p
   gate. Pop frame on breakpoint hit.
   (_initialize_infcall): Add add_setshow_boolean_cmd for
   unwind-on-terminating-exception.

gdb/testsuite/ChangeLog
2008-10-13  Phil Muldoon  <pmuldoon@redhat.com>

   * gdb.cp/gdb2495.cc: New file.
   * gdb.cp/gdb2495.exp: New file.


diff --git a/gdb/infcall.c b/gdb/infcall.c
index 5cc068a..95e99ea 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -91,6 +91,29 @@ Unwinding of stack if a signal is received while in a call dummy is %s.\n"),
 		    value);
 }
 
+/* This boolean tells what gdb should do if a std::terminate call is
+   made while in a function called from gdb (call dummy).  
+   As the confines of a single dummy stack prohibit out-of-frame
+   handlers from handling a raised exception, and as out-of-frame
+   handlers are common in C++, this can lead to no handler being found
+   by the unwinder, and a std::terminate call. This is a false positive.
+   If set, gdb unwinds the stack and restores the context to what it 
+   was before the call.
+
+   The default is to unwind the frame if a std::terminate call is made.. */
+
+int unwind_on_terminating_exception_p = 1;
+static void
+show_unwind_on_terminating_exception_p (struct ui_file *file, int from_tty,
+					struct cmd_list_element *c,
+					const char *value)
+
+{
+  fprintf_filtered (file, _("\
+Unwinding of stack from a std::terminate call originating from \
+default C++ exception handler is %s.\n"),
+		    value);
+}
 
 /* Perform the standard coercions that are specified
    for arguments to be passed to C or Ada functions.
@@ -328,6 +351,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 = 0;
+  struct minimal_symbol *tm;
 
   if (TYPE_CODE (ftype) == TYPE_CODE_PTR)
     ftype = check_typedef (TYPE_TARGET_TYPE (ftype));
@@ -652,6 +677,26 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   /* Now proceed, having reached the desired place.  */
   clear_proceed_status ();
     
+  /* Create a breakpoint in std::terminate. 
+     If a C++ exception is raised in the dummy-frame, and the 
+     exception handler is (normally, and expected to be) out-of-frame, 
+     the default C++ handler will (wrongly) be called in an inferior
+     function call.  This is wrong, as an exception can be  normally
+     and legally handled out-of-frame. The confines of the dummy frame
+     prevent the unwinder from finding the correct handler (or any
+     handler, unless it is in-frame). The default handler calls
+     std::terminate. This will kill the inferior. Assert that
+     terminate should never be called in an inferior function
+     call. Place a momentary breakpoint in the std::terminate function
+     and if triggered in the call, rewind */
+  if (unwind_on_terminating_exception_p)
+    {
+      tm = lookup_minimal_symbol ("std::terminate()", NULL, NULL); 
+      if (tm != NULL)
+	terminate_bp = set_momentary_breakpoint_at_pc
+	  (SYMBOL_VALUE_ADDRESS (tm),  bp_breakpoint);
+    }
+  
   /* Execute a "stack dummy", a piece of code stored in the stack by
      the debugger to be executed in the inferior.
 
@@ -699,6 +744,10 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
     discard_cleanups (old_cleanups);
   }
 
+  /* Only clean up terminating exception breakpoint if it was set */
+  if (terminate_bp != NULL)
+    make_cleanup_delete_breakpoint (terminate_bp);
+ 
   if (stopped_by_random_signal || !stop_stack_dummy)
     {
       /* Find the name of the function we're about to complain about.  */
@@ -778,6 +827,30 @@ Evaluation of the expression containing the function (%s) will be abandoned."),
 	     previously selected frame), would write the registers
 	     from the inf_status (which is wrong), and would do other
 	     wrong things.  */
+
+	  /* 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)) 
+		
+		
+		{
+		  frame_pop (get_current_frame ());
+		  error (_("\
+The program being debugged entered a std::terminate call which would\n\
+have terminated the program being debugged.  GDB has restored the\n\
+context to what it was before the call.\n\
+To change this behaviour use \"set unwind-on-terminating-exception \
+off\"\n\
+Evaluation of the expression containing the function (%s) will be abandoned."), name);
+		}
+	    } 
 	  discard_cleanups (inf_status_cleanup);
 	  discard_inferior_status (inf_status);
 	  /* The following error message used to say "The expression
@@ -878,4 +951,25 @@ The default is to stop in the frame where the signal was received."),
 			   NULL,
 			   show_unwind_on_signal_p,
 			   &setlist, &showlist);
+
+  add_setshow_boolean_cmd ("unwind-on-terminating-exception", no_class,
+			   &unwind_on_terminating_exception_p, _("\
+Set unwinding of stack if a std::terminate() call originates from\n\
+the default C++ exception handler."), _("\
+Show unwinding of stack if a std::terminate() call originates from\n\
+the default C++ exception handler."), _("\
+The unwind-on-terminating-exception lets the user determine what\n\
+gdb should do if a std::terminate() call is made from the default\n\
+exception handler due to no in-frame handler in the dummy frame.\n\
+C++ exceptions are often handled out-of-frame, but the constraints\n\
+of the call-dummy can fool the unwinder into thinking there is no\n\
+exception handler, and calls the default handler. This in turns\n\
+calls std::terminate, which will terminate the inferior.\n\
+If set, gdb unwinds the stack and restores the context to what it\n\
+was before the call.\n\
+The default is to unwind the frame."),
+			   NULL,
+			   show_unwind_on_terminating_exception_p,
+			   &setlist, &showlist);
+
 }
diff --git a/gdb/testsuite/gdb.cp/gdb2495.cc b/gdb/testsuite/gdb.cp/gdb2495.cc
new file mode 100644
index 0000000..4df265f
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/gdb2495.cc
@@ -0,0 +1,90 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008 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 <iostream>
+#include <signal.h>
+
+using namespace std;
+
+class SimpleException 
+{
+
+public:
+
+  void raise_signal (int dummy) 
+  {
+    if (dummy > 0)
+      raise(SIGABRT);
+  }
+
+  int  no_throw_function () 
+  {
+    return 1;
+  }
+  
+  void throw_function () 
+  {
+    throw 1;
+  }
+  
+  int throw_function_with_handler () 
+  {
+    try
+      {
+	throw 1;
+      }
+    catch (...)
+      {
+	cout << "Handled" << endl;
+      } 
+    
+    return 2;
+  }
+  
+  void call_throw_function_no_handler () 
+  {
+    throw_function ();
+  }
+  
+  void call_throw_function_handler () 
+  {
+    throw_function_with_handler ();
+  }
+};
+SimpleException exceptions;
+
+int 
+main()
+{
+  // Have to call all these functions
+  // so not optimized away.
+  exceptions.raise_signal (-1);
+  exceptions.no_throw_function ();
+  exceptions.throw_function_with_handler ();
+  exceptions.call_throw_function_handler ();
+  try 
+    {
+      exceptions.throw_function ();
+      exceptions.call_throw_function_no_handler ();
+    }
+  catch (...)
+    {
+    }
+  return 0;
+}
+
diff --git a/gdb/testsuite/gdb.cp/gdb2495.exp b/gdb/testsuite/gdb.cp/gdb2495.exp
new file mode 100644
index 0000000..beb744c
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/gdb2495.exp
@@ -0,0 +1,160 @@
+# Copyright 2008 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/>.
+
+
+# In gdb inferior function calls, if a C++ exception is raised in the
+# dummy-frame, and the exception handler is (normally, and expected to
+# be) out-of-frame, the default C++ handler will (wrongly) be called
+# in an inferior function call.  
+# This is incorrect as an exception can normally and legally be handled
+# out-of-frame. The confines of the dummy frame prevent the unwinder
+# from finding the correct handler (or any handler, unless it is
+# in-frame). The default handler calls std::terminate. This will kill
+# the inferior. Assert that terminate should never be called in an
+# inferior function call. These tests test the functionality around
+# unwinding that sequence and also tests the flag behaviour gating this
+# functionality.
+
+# This test is largley based off gdb.base/callfuncs.exp.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+if { [skip_cplus_tests] } { continue }
+
+set prms_id 2495
+set bug_id 0
+
+set testfile "gdb2495"
+set srcfile ${testfile}.cc
+set binfile $objdir/$subdir/$testfile
+
+# Create and source the file that provides information about the compiler
+# used to compile the test case.
+if [get_compiler_info ${binfile} "c++"] {
+    return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+     untested gdb2495.exp
+     return -1
+}
+
+# Some targets can't do function calls, so don't even bother with this
+# test.
+if [target_info exists gdb,cannot_call_functions] {
+    setup_xfail "*-*-*" 2416
+    fail "This target can not call functions"
+    continue
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to main"
+    continue
+} 
+
+# See http://sources.redhat.com/gdb/bugs/2495
+
+# Test normal baseline behaviour. Call a function that
+# does not raise an exception ...
+gdb_test "p exceptions.no_throw_function()" " = 1"
+# And one that does but handles it in-frame ...
+gdb_test "p exceptions.throw_function_with_handler()" " = 2"
+# Both should return normally.
+
+# Test basic unwind. Call a function that raises an exception but
+# does not handle it. It should be rewound ...
+gdb_test "p exceptions.throw_function()" \
+    "The program being debugged entered a std::terminate call .*" \
+    "Call a function that raises an exception without a handler."
+
+# Make sure that after rewinding we are back at the call parent.
+gdb_test "bt" \
+    "#0  main.*" \
+    "bt after returning from a popped frame"
+
+# Make sure the only breakpoint is the one set via the runto_main
+# call and that the std::terminate breakpoint has evaporated and
+# cleaned-up.
+gdb_test "info breakpoints" \
+    "gdb.cp/gdb2495\.cc.*"
+
+# Turn off this new behaviour ...
+send_gdb "set unwind-on-terminating-exception off\n" 
+gdb_expect {
+    -re "$gdb_prompt $" {pass "set unwind-on-terminating-exception"}
+    timeout {fail "(timeout) set  unwind-on-terminating-exception"}
+}
+
+# Check that it is turned off ...
+gdb_test "show unwind-on-terminating-exception" \
+    "exception handler is off.*" \
+    "Turn off unwind on terminating exception flag"
+
+# Check that the old behaviour is restored.
+gdb_test "p exceptions.throw_function()" \
+    "The program being debugged was signaled while in a function .*" \
+    "Call a function that raises an exception with unwinding off.."
+
+
+# Restart back at main
+if ![runto_main] then {
+    perror "couldn't run to main"
+    continue
+} 
+
+
+# Check to see if our new behaviour alters the unwind signal
+# behaviour. It should not. Test both on and off states.
+
+# Turn on unwind on signal  behaviour ...
+send_gdb "set unwindonsignal on\n" 
+gdb_expect {
+    -re "$gdb_prompt $" {pass "set unwindonsignal on"}
+    timeout {fail "(timeout) set  unwindonsignal on"}
+}
+
+# Check that it is turned on ...
+gdb_test "show unwindonsignal" \
+    "signal is received while in a call dummy is on.*" \
+    "Turn on unwind on signal"
+
+# Check to see if new behaviour interferes with
+# normal signal handling in inferior function calls.
+gdb_test "p exceptions.raise_signal(1)" \
+    "To change this behavior use \"set unwindonsignal off\".*"
+
+# And reverse. Turn off
+send_gdb "set unwindonsignal off\n" 
+gdb_expect {
+    -re "$gdb_prompt $" {pass "set unwindonsignal off"}
+    timeout {fail "(timeout) set  unwindonsignal off"}
+}
+
+# Check that it is turned off ...
+gdb_test "show unwindonsignal" \
+    "signal is received while in a call dummy is off.*" \
+    "Turn off unwind on signal"
+
+# Check to see if new behaviour interferes with
+# normal signal handling in inferior function calls.
+gdb_test "p exceptions.raise_signal(1)" \
+    "To change this behavior use \"set unwindonsignal on\".*"