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

Hui Zhu
2013-05-14T11:15:42Z

[-- 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\"." \