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

Hui Zhu
2013-05-13T08:24:20Z

Hi Yao,

Thanks for your review.

On Mon, May 13, 2013 at 3:13 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 05/11/2013 01:54 PM, Hui Zhu wrote:
>>
>> @@ -2302,6 +2302,9 @@ build_target_command_list (struct bp_loc
>> need to parse the command to bytecodes again. */
>> aexpr = parse_cmd_to_aexpr (bl->address,
>> loc->owner->extra_string);
>> + if (aexpr == NULL)
>> + error (_("Agent is not support commands of breakpoint
>> %d."),
>> + bl->owner->number);
>> loc->cmd_bytecode = aexpr;
>>
>> if (!aexpr)
>
>
> The existing code has considered the situation that we have a NULL bytecode
> expression. Several lines of code below this line of patch do something,
>
> /* If we have a NULL bytecode expression, it means something
> went wrong or we have a null command expression. */
> if (!loc->cmd_bytecode)
> {
> null_command_or_parse_error = 1;
> break;
> }
> }
> }
>
> /* If anything failed, then we're not doing target-side commands,
> and so clean up. */
> if (null_command_or_parse_error)
>
> Your patch doesn't look right, and probably we need to examine why dprintf
> breakpoint is not handled well in this part.

I think the design of current build_target_command_list is not right.
It use null_command_or_parse_error that close with
build_target_condition_list.

For the condition, the commands of agent is same with gdb side. So
when it got fail or NULL. Agent can go back to let GDB handle this
condition. So it have null_condition_or_parse_error.

But for commands, the commands of agent is not support by GDB side
(agent-printf). So even if it go back to GDB, it still cannot be
handle. So I think throw error in build_target_command_list is
better.

I will post patch to fix it later.

Thanks,
Hui