From: Hui Zhu <teawater@gmail.com>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches ml <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Fix bug 15433 - GDB crashes when using agent dprintf, %s format, and an in-line string
Date: Mon, 13 May 2013 08:24:00 -0000 [thread overview]
Message-ID: <CANFwon03jqPwCJAC50SdqBDAfis_OAFkcrNWsO9WLRd1u17mAg@mail.gmail.com> (raw)
In-Reply-To: <51909285.6070008@codesourcery.com>
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
next prev parent reply other threads:[~2013-05-13 8:24 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-11 5:55 Hui Zhu
2013-05-13 7:13 ` Yao Qi
2013-05-13 8:24 ` Hui Zhu [this message]
2013-05-13 16:36 ` Tom Tromey
2013-05-14 11:15 ` Hui Zhu
2013-07-17 20:22 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CANFwon03jqPwCJAC50SdqBDAfis_OAFkcrNWsO9WLRd1u17mAg@mail.gmail.com \
--to=teawater@gmail.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox