Re: [PATCH] Fix bug 15433 - GDB crashes when using agent dprintf, %s format, and an in-line string

[Hui Zhu <teawater@gmail.com>]

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

Hi Tom,

Thanks for your review.

On Tue, May 14, 2013 at 12:35 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>
> Hui> +        if (aexpr == NULL)
> Hui> +          error (_("Agent is not support commands of breakpoint %d."),
> Hui> +                 bl->owner->number);
>
> In addition to what Yao said, this change seems roughly equivalent to
> simply removing the TRY_CATCH from parse_cmd_to_aexpr.  (You'd have to
> clear loc->cmd_bytecode first, but it seems to me that this has to be
> done anyway, because parse_cmd_to_aexpr can already call error...)

Agree with you.  Updated patches for it.

>
> I also still don't understand the code to set null_command_or_parse_error.
> Maybe you explained it to me last time, but I forget.  If it is actually
> correct as-is, I would appreciate a comment explaining it.

I posted some introduction about it in
http://sourceware.org/ml/gdb-patches/2013-05/msg00425.html .  I hope
it will be helpful to you to understand this function.
And I suggest we can discussion this function in thread
http://sourceware.org/ml/gdb-patches/2013-04/msg00839.html ([PATCH/v2]
fix Bug 15180 Agent style dprintf does not respect conditions, I will
ping it later) that I did a lot of change about this function.

Thanks,
Hui

2013-05-14  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15433
	* breakpoint.c (parse_cmd_to_aexpr): Remove TRY_CATCH for gen_printf.

2013-05-14  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15433
	* gdb.base/dprintf.exp: Test unsupport commands on target.

[-- Attachment #2: dprintf-cmd-not-support.txt --]
[-- Type: text/plain, Size: 828 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2239,22 +2239,9 @@ parse_cmd_to_aexpr (CORE_ADDR scope, cha
 	++cmdrest;
     }
 
-  /* We don't want to stop processing, so catch any errors
-     that may show up.  */
-  TRY_CATCH (ex, RETURN_MASK_ERROR)
-    {
-      aexpr = gen_printf (scope, gdbarch, 0, 0,
-			  format_start, format_end - format_start,
-			  fpieces, nargs, argvec);
-    }
-
-  if (ex.reason < 0)
-    {
-      /* If we got here, it means the command could not be parsed to a valid
-	 bytecode expression and thus can't be evaluated on the target's side.
-	 It's no use iterating through the other commands.  */
-      return NULL;
-    }
+  aexpr = gen_printf (scope, gdbarch, 0, 0,
+		      format_start, format_end - format_start,
+		      fpieces, nargs, argvec);
 
   do_cleanups (old_cleanups);
 

[-- Attachment #3: dprintf-cmd-not-support-test.txt --]
[-- Type: text/plain, Size: 531 bytes --]

--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -131,6 +131,11 @@ if $target_can_dprintf {
 	"\[\r\n\]\tbreakpoint already hit 2 times"
 	"\[\r\n\]        agent-printf \"arg=%d, g=%d\\\\n\", arg, g"
     }
+
+    # Test unsupport commands.
+    gdb_test "dprintf $dp_location1,\"%s\\n\", \"test\"" "Dprintf .*"
+    gdb_test "continue" ".*Unsupported operator OP_STRING .* in expression.*" \
+      "3rd dprintf, agent"
 }
 
 gdb_test "set dprintf-style foobar" "Undefined item: \"foobar\"." \