[RFA] Rethrow exception properly.

[RFA] Rethrow exception properly.

[Vladimir Prus]

Nick has reported that my recent changes regressed MI
error messages, and now we get:

  (gdb) 
  -break-insert sqrt
  &"Function \"sqrt\" not defined.\n"
  ^error,msg="unknown error"
  (gdb) 

The problem is in rather old code in  break_command_really -- 
when we can't parse breakpoint location, and pending breakpoints 
are off, we rethrow the exception using the deprecated_throw_reason.
Unfortunately, that strips the message from the exception, 
so MI code gets rather useless exception.

Such rethrowing is just wrong -- if we fully delegate handling of
the error to our caller, we should just throw the original exception.

The attached patch switches to using throw_exception and gets us
back to sane MI error message. OK?

- Volodya


	[gdb]
        Properly rethrow exception.  This fixes errors
	about non-existent functions for -break-insert.

	* breakpoint.c (break_command_really): Use throw_exception
	for rethrowing.  If rethrowing, don't print the exception.

	[gdb/testsuite]
	* gdb.mi/mi-break.exp (test_error): New.
	Call it.
---
 gdb/breakpoint.c                  |    6 +++---
 gdb/testsuite/gdb.mi/mi-break.exp |   10 ++++++++++
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 9551bd5..d5de3e7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -5280,13 +5280,13 @@ break_command_really (char *arg, char *cond_string, int thread,
 	{
 	case NOT_FOUND_ERROR:
 
-	  exception_print (gdb_stderr, e);
-
 	  /* If pending breakpoint support is turned off, throw
 	     error.  */
 
 	  if (pending_break_support == AUTO_BOOLEAN_FALSE)
-	    deprecated_throw_reason (RETURN_ERROR);
+	    throw_exception (e);
+
+	  exception_print (gdb_stderr, e);
 
           /* If pending breakpoint support is auto query and the user
 	     selects no, then simply return the error code.  */
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index af438b9..ca59348 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -175,10 +175,20 @@ proc test_ignore_count {} {
     }  
 }
 
+proc test_error {} {
+    global mi_gdb_prompt
+
+    mi_gdb_test "-break-insert function_that_does_not_exit" \
+        ".*\\^error,msg=\"Function \\\\\"function_that_does_not_exit\\\\\" not defined.\"" \
+        "breakpoint at nonexistent function"
+}
+
 test_tbreak_creation_and_listing
 test_rbreak_creation_and_listing
 
 test_ignore_count
 
+test_error
+
 mi_gdb_exit
 return 0
-- 
1.5.3.5

Re: [RFA] Rethrow exception properly.

[Nick Roberts]

 > Such rethrowing is just wrong -- if we fully delegate handling of
 > the error to our caller, we should just throw the original exception.
 > 
 > The attached patch switches to using throw_exception and gets us
 > back to sane MI error message. OK?

There is still the anomaly between break_command_really returning int and
gdb_breakpoint returning enum gdb_rc.  Currently it looks like the return value
of break_command_1 is not used, so this could presumably be void.  If the
exception handling is all done in break_command_really perhaps we don't need
the return value of gdb_breakpoint.

 >...
 > +proc test_error {} {
 > +    global mi_gdb_prompt
 > +
 > +    mi_gdb_test "-break-insert function_that_does_not_exit" \
                                   function_that_does_not_exist

 > +        ".*\\^error,msg=\"Function \\\\\"function_that_does_not_exit\\\\\" not defined.\"" \
                                             function_that_does_not_exist

 > +        "breakpoint at nonexistent function"
 > +}
 >...

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [RFA] Rethrow exception properly.

[Vladimir Prus]

On Sunday 27 January 2008 05:18:32 Nick Roberts wrote:
>  > Such rethrowing is just wrong -- if we fully delegate handling of
>  > the error to our caller, we should just throw the original exception.
>  > 
>  > The attached patch switches to using throw_exception and gets us
>  > back to sane MI error message. OK?
> 
> There is still the anomaly between break_command_really returning int and
> gdb_breakpoint returning enum gdb_rc.  Currently it looks like the return value
> of break_command_1 is not used, so this could presumably be void.  If the
> exception handling is all done in break_command_really perhaps we don't need
> the return value of gdb_breakpoint.

That's a topic of separate patch (which will fix yet another problem). This
patch fixes the first apparent issue.

- Volodya

Re: [RFA] Rethrow exception properly.

[Daniel Jacobowitz]

On Sat, Jan 26, 2008 at 10:57:35PM +0300, Vladimir Prus wrote:
> 	[gdb]
>         Properly rethrow exception.  This fixes errors
> 	about non-existent functions for -break-insert.
> 
> 	* breakpoint.c (break_command_really): Use throw_exception
> 	for rethrowing.  If rethrowing, don't print the exception.
> 
> 	[gdb/testsuite]
> 	* gdb.mi/mi-break.exp (test_error): New.
> 	Call it.

This is OK (with the spelling correction for
function_that_does_not_exist).

-- 
Daniel Jacobowitz
CodeSourcery