[PATCH] Make AX engine fail gracefully due to unknown types

[PATCH] Make AX engine fail gracefully due to unknown types

[Luis Gustavo]

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

Hi,

The target-side conditionals patch exposes the AX engine to a wider 
range of use cases, and it does not seem to be prepared to handle all 
those right now.

One example is handling of unsupported types, like float. The proposed 
fix makes the AX engine fail gracefully when it sees unknown/unsupported 
types. It does this without compromising the debugging session like an 
internal error.

This fixes the internal error seen in gdb.cp/mb-templates.exp, where we 
have two locations for a single conditional breakpoint (i==1). The 
problem here is that i is INT for one of the locations and FLOAT for the 
other.

With the fix, GDB handles those two locations' conditions appropriately.

I've also reverted the KFAIL workaround for mb-templates.exp.

Regtested in X86.

Ok?

[-- Attachment #2: bz13781.diff --]
[-- Type: text/x-patch, Size: 2860 bytes --]

2012-03-07  Luis Machado  <lgustavo@codesourcery.com>

	* ax-gdb.c (gen_fetch): Fail gracefully and use error instead
	of internal error for unknown/unsupported types.

	Revert:

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

	Implement testsuite workaround for PR breakpoints/13781.
	* gdb.cp/mb-templates.exp: New loop with variable $workaround.
	(set breakpoint condition-evaluation host): New conditional command.

Index: HEAD-git/gdb/ax-gdb.c
===================================================================
--- HEAD-git.orig/gdb/ax-gdb.c	2012-03-07 10:07:48.186611086 -0300
+++ HEAD-git/gdb/ax-gdb.c	2012-03-07 14:16:23.846145476 -0300
@@ -557,8 +557,8 @@ gen_fetch (struct agent_expr *ax, struct
          pointer (other code's fault), or we're not implementing
          something we should be (this code's fault).  In any case,
          it's a bug the user shouldn't see.  */
-      internal_error (__FILE__, __LINE__,
-		      _("gen_fetch: bad type code"));
+      error (_("gen_fetch: Unsupported type code `%s'."),
+	     TYPE_NAME (type));
     }
 }
 
Index: HEAD-git/gdb/testsuite/gdb.cp/mb-templates.exp
===================================================================
--- HEAD-git.orig/gdb/testsuite/gdb.cp/mb-templates.exp	2012-03-07 10:08:11.218610354 -0300
+++ HEAD-git/gdb/testsuite/gdb.cp/mb-templates.exp	2012-03-07 14:11:03.734155471 -0300
@@ -38,35 +38,24 @@ if  { [gdb_compile "${srcdir}/${subdir}/
      return -1
 }
 
-# If GDB crashes try it again workarounding PR breakpoints/13781.
+clean_restart ${executable}
 
-foreach workaround [list "" " (workaround)"] {
-    clean_restart ${executable}
+set bp_location [gdb_get_line_number "set breakpoint here"]
 
-    if {${workaround} != ""} {
-	gdb_test_no_output "set breakpoint condition-evaluation host"
-    }
-
-    set bp_location [gdb_get_line_number "set breakpoint here"]
+# Set a breakpoint with multiple locations
+# and a condition.
 
-    # Set a breakpoint with multiple locations
-    # and a condition.
+gdb_test "break $srcfile:$bp_location if i==1" \
+    "Breakpoint.*at.*: $srcfile:$bp_location. \\(2 locations\\).*" \
+    "initial condition: set breakpoint"
 
-    gdb_test "break $srcfile:$bp_location if i==1" \
-	"Breakpoint.*at.*: $srcfile:$bp_location. \\(2 locations\\).*" \
-	"initial condition: set breakpoint$workaround"
+gdb_run_cmd
 
-    gdb_run_cmd
-
-    if {${workaround} == "" && [is_remote target]} {
-	setup_kfail breakpoints/13781 "*-*-*"
-    }
-    set test "initial condition: run to breakpoint$workaround"
-    gdb_test_multiple "" $test {
-	-re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
-	    pass $test
-	    break
-	}
+set test "initial condition: run to breakpoint"
+gdb_test_multiple "" $test {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass $test
+	break
     }
 }
 

Re: [PATCH] Make AX engine fail gracefully due to unknown types

[Luis Gustavo]

Obviously i forgot to mention that this fixes PR 13781... :-)


On 03/07/2012 03:22 PM, Luis Gustavo wrote:
> Hi,
>
> The target-side conditionals patch exposes the AX engine to a wider
> range of use cases, and it does not seem to be prepared to handle all
> those right now.
>
> One example is handling of unsupported types, like float. The proposed
> fix makes the AX engine fail gracefully when it sees unknown/unsupported
> types. It does this without compromising the debugging session like an
> internal error.
>
> This fixes the internal error seen in gdb.cp/mb-templates.exp, where we
> have two locations for a single conditional breakpoint (i==1). The
> problem here is that i is INT for one of the locations and FLOAT for the
> other.
>
> With the fix, GDB handles those two locations' conditions appropriately.
>
> I've also reverted the KFAIL workaround for mb-templates.exp.
>
> Regtested in X86.
>
> Ok?

Re: [PATCH] Make AX engine fail gracefully due to unknown types

[Jan Kratochvil]

On Wed, 07 Mar 2012 19:22:54 +0100, Luis Gustavo wrote:
> +++ HEAD-git/gdb/ax-gdb.c	2012-03-07 14:16:23.846145476 -0300
> @@ -557,8 +557,8 @@ gen_fetch (struct agent_expr *ax, struct
>           pointer (other code's fault), or we're not implementing
>           something we should be (this code's fault).  In any case,
>           it's a bug the user shouldn't see.  */
> -      internal_error (__FILE__, __LINE__,
> -		      _("gen_fetch: bad type code"));
> +      error (_("gen_fetch: Unsupported type code `%s'."),
> +	     TYPE_NAME (type));

I find it OK but here the comment should be updated.


Thanks,
Jan

Re: [PATCH] Make AX engine fail gracefully due to unknown types

[Luis Gustavo]

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

On 03/07/2012 05:22 PM, Jan Kratochvil wrote:
> On Wed, 07 Mar 2012 19:22:54 +0100, Luis Gustavo wrote:
>> +++ HEAD-git/gdb/ax-gdb.c	2012-03-07 14:16:23.846145476 -0300
>> @@ -557,8 +557,8 @@ gen_fetch (struct agent_expr *ax, struct
>>            pointer (other code's fault), or we're not implementing
>>            something we should be (this code's fault).  In any case,
>>            it's a bug the user shouldn't see.  */
>> -      internal_error (__FILE__, __LINE__,
>> -		      _("gen_fetch: bad type code"));
>> +      error (_("gen_fetch: Unsupported type code `%s'."),
>> +	     TYPE_NAME (type));
>
> I find it OK but here the comment should be updated.
>
>
> Thanks,
> Jan

How about this one?

Luis

[-- Attachment #2: bz13781.diff --]
[-- Type: text/x-patch, Size: 3130 bytes --]

2012-03-07  Luis Machado  <lgustavo@codesourcery.com>

	* ax-gdb.c (gen_fetch): Fail gracefully and use error instead
	of internal error for unknown/unsupported types.

	Revert:

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

	Implement testsuite workaround for PR breakpoints/13781.
	* gdb.cp/mb-templates.exp: New loop with variable $workaround.
	(set breakpoint condition-evaluation host): New conditional command.

Index: HEAD-git/gdb/ax-gdb.c
===================================================================
--- HEAD-git.orig/gdb/ax-gdb.c	2012-03-07 14:59:50.546064105 -0300
+++ HEAD-git/gdb/ax-gdb.c	2012-03-08 08:41:25.976898540 -0300
@@ -553,12 +553,11 @@ gen_fetch (struct agent_expr *ax, struct
       break;
 
     default:
-      /* Either our caller shouldn't have asked us to dereference that
-         pointer (other code's fault), or we're not implementing
-         something we should be (this code's fault).  In any case,
-         it's a bug the user shouldn't see.  */
-      internal_error (__FILE__, __LINE__,
-		      _("gen_fetch: bad type code"));
+      /* Our caller requested us to dereference a pointer from an unsupported
+	 type.  Error out and give callers a chance to handle the failure
+	 gracefully.  */
+      error (_("gen_fetch: Unsupported type code `%s'."),
+	     TYPE_NAME (type));
     }
 }
 
Index: HEAD-git/gdb/testsuite/gdb.cp/mb-templates.exp
===================================================================
--- HEAD-git.orig/gdb/testsuite/gdb.cp/mb-templates.exp	2012-03-07 14:59:50.522064107 -0300
+++ HEAD-git/gdb/testsuite/gdb.cp/mb-templates.exp	2012-03-08 08:27:48.344944130 -0300
@@ -38,35 +38,24 @@ if  { [gdb_compile "${srcdir}/${subdir}/
      return -1
 }
 
-# If GDB crashes try it again workarounding PR breakpoints/13781.
+clean_restart ${executable}
 
-foreach workaround [list "" " (workaround)"] {
-    clean_restart ${executable}
+set bp_location [gdb_get_line_number "set breakpoint here"]
 
-    if {${workaround} != ""} {
-	gdb_test_no_output "set breakpoint condition-evaluation host"
-    }
-
-    set bp_location [gdb_get_line_number "set breakpoint here"]
+# Set a breakpoint with multiple locations
+# and a condition.
 
-    # Set a breakpoint with multiple locations
-    # and a condition.
+gdb_test "break $srcfile:$bp_location if i==1" \
+    "Breakpoint.*at.*: $srcfile:$bp_location. \\(2 locations\\).*" \
+    "initial condition: set breakpoint"
 
-    gdb_test "break $srcfile:$bp_location if i==1" \
-	"Breakpoint.*at.*: $srcfile:$bp_location. \\(2 locations\\).*" \
-	"initial condition: set breakpoint$workaround"
+gdb_run_cmd
 
-    gdb_run_cmd
-
-    if {${workaround} == "" && [is_remote target]} {
-	setup_kfail breakpoints/13781 "*-*-*"
-    }
-    set test "initial condition: run to breakpoint$workaround"
-    gdb_test_multiple "" $test {
-	-re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
-	    pass $test
-	    break
-	}
+set test "initial condition: run to breakpoint"
+gdb_test_multiple "" $test {
+    -re "Breakpoint \[0-9\]+,.*foo<int> \\(i=1\\).*$gdb_prompt $" {
+	pass $test
+	break
     }
 }
 

Re: [PATCH] Make AX engine fail gracefully due to unknown types

[Jan Kratochvil]

On Thu, 08 Mar 2012 12:47:38 +0100, Luis Gustavo wrote:
> How about this one?

Fine with me for check-in.


Thanks,
Jan

Re: [PATCH] Make AX engine fail gracefully due to unknown types

[Luis Gustavo]

On 03/08/2012 01:52 PM, Jan Kratochvil wrote:
> On Thu, 08 Mar 2012 12:47:38 +0100, Luis Gustavo wrote:
>> How about this one?
>
> Fine with me for check-in.
>
>
> Thanks,
> Jan

Thanks! I've checked this in now.

Luis