Re: [PATCH/v2] fix Bug 15180 Agent style dprintf does not respect conditions

[Hui Zhu <teawater at gmail dot com>]

I am so sorry that the dprintf-remote-cond-server.txt is not the right version.

I post a new version to fix that.

Thanks,
Hui

2013-07-01  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* breakpoint.c (build_target_condition_list,
	build_target_command_list): Removed.
	(build_target_condition_list_first, build_target_command_list_first,
	build_target_command_list_and_condition_list): New functions.
	(insert_bp_location): Call
	build_target_command_list_and_condition_list.
	* remote.c (remote_add_target_side_condition): Handle NULL.
	(remote_add_target_side_commands): Ditto.

2013-07-01  Hui Zhu  <hui@codesourcery.com>

	PR gdb/15180
	* linux-low.c (linux_wait_1): Update arguments of function
	gdb_condition_true_at_breakpoint.
	Remove function run_breakpoint_commands.
	(need_step_over_p): Update arguments of function
	gdb_condition_true_at_breakpoint.
	* mem-break.c (add_condition_to_breakpoint): Add new condition
	to tail.
	(gdb_condition_true_at_breakpoint): Execute commands.
	(add_commands_to_breakpoint): Add new commands to tail.
	* mem-break.h (gdb_condition_true_at_breakpoint): Update arguments.

On Mon, Jun 17, 2013 at 3:35 PM, Hui Zhu <teawater@gmail.com> wrote:
> Hi,
>
> I found that these patches cannot pass the test of dprintf.  And it
> have something wrong.  So I make a new version for it.
>
> And the last issue that I told about "May only run agent-printf on the
> target".  Because patch for it depend on the patches in
> http://sourceware.org/ml/gdb-patches/2013-06/msg00139.html.  So I will
> post it in another thread.
>
> Thanks,
> Hui
>
> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
>
>         PR gdb/15180
>         * breakpoint.c (build_target_condition_list,
>         build_target_command_list): Removed.
>         (build_target_condition_list_first, build_target_command_list_first,
>         build_target_command_list_and_condition_list): New functions.
>         (insert_bp_location): Call
>         build_target_command_list_and_condition_list.
>         * remote.c (remote_add_target_side_condition): Handle NULL.
>         (remote_add_target_side_commands): Ditto.
>
> 2013-06-17  Hui Zhu  <hui@codesourcery.com>
>
>         PR gdb/15180
>         * linux-low.c (linux_wait_1): Update arguments of function
>         gdb_condition_true_at_breakpoint.
>         Remove function run_breakpoint_commands.
>         (need_step_over_p): Update arguments of function
>         gdb_condition_true_at_breakpoint.
>         * mem-break.c (add_condition_to_breakpoint): Add new condition
>         to tail.
>         (gdb_condition_true_at_breakpoint): Execute commands.
>         (add_commands_to_breakpoint): Add new commands to tail.
>         * mem-break.h (gdb_condition_true_at_breakpoint): Update arguments.
>
>
>
> On Sun, Apr 28, 2013 at 4:33 PM, Hui Zhu <teawater@gmail.com> wrote:
>> Hi,
>>
>> As the Yao said in the first version patch, the core issue of this bug
>> is the command is not well grouped or associated with condition in
>> target side.
>> And after I check the code of GDB, I found that in gdb part, when it
>> build target commands list (build_target_command_list) and condition
>> list (build_target_condition_list) to bl->target_info, they do not
>> good grouped commands and condition.  Also they have another issue for
>> example, not handle disable breakpoints very well.
>>
>>
>> So I merge the two functions together to function
>> build_target_command_list_and_condition_list.
>> It will do check for all the commands and conditions in a address.
>> After that, push commands and conditions bytecodes to target_info.  It
>> will push NULL to it if need.
>> With this NULL, because commands and conditions have the same number,
>> then it can be grouped.
>> In the end of this function, it will pust a NULL conditions, it can
>> handle the BL has commands on target but always need let GDB handle
>> the conditions.
>>
>> After that, I updated remote_add_target_side_commands and
>> remote_add_target_side_condition.  Then if it got NULL condition or
>> commands.   It will send bytecodes with 0 size to gdbserver.
>>
>>
>> In gdbserver part, I update it add commands and condition in right
>> order like how it received because maybe we have a 0 size conditions
>> in the end without commands.
>> Tom suggest we handle commands of dprintf together with condition
>> check to handle bug 15075 in
>> http://sourceware.org/ml/gdb-patches/2013-02/msg00557.html
>> I think this is a good idea to handle commands with conditions, so I
>> post patch http://sourceware.org/ml/gdb-patches/2013-04/msg00711.html
>> for 15075.
>> And I do same thing in gdbserver.  I update function
>> gdb_condition_true_at_breakpoint let it can execute commands too if
>> need.
>>
>>
>> In the end, after these patches, gdb still have issue if a breakpoints
>> with conditions and dprintf with condions:
>> (gdb) target remote :1234
>> Remote debugging using :1234
>> Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging
>> symbols found)...done.
>> Loaded symbols for /lib64/ld-linux-x86-64.so.2
>> 0x00007ffff7ddb6b0 in ?? () from /lib64/ld-linux-x86-64.so.2
>> (gdb) set dprintf-style
>> agent  call   gdb
>> (gdb) set dprintf-style agent
>> (gdb) source 2
>> Dprintf 1 at 0x4005a7: file 2.c, line 7.
>> Breakpoint 2 at 0x4005a7: file 2.c, line 7.
>> (gdb) info b
>> Num     Type           Disp Enb Address            What
>> 1       dprintf        keep y   0x00000000004005a7 in main at 2.c:7
>>         stop only if i>3 (host evals)
>>         agent-printf "%d\n",i
>> 2       breakpoint     keep y   0x00000000004005a7 in main at 2.c:7
>>         stop only if i>4 (host evals)
>> (gdb) c
>> Continuing.
>>
>> Breakpoint 1, main () at 2.c:7
>> 7          printf("friend\n");
>> May only run agent-printf on the target
>>
>> But I think this issue is about howto handle dprintf commands in the
>> gdb part, and I have another patch about it is waiting review.  So I
>> will post patch for it later.
>>
>> Thanks,
>> Hui
>>
>> 2013-04-27  Hui Zhu  <hui@codesourcery.com>
>>
>>         PR gdb/15180
>>         * breakpoint.c (build_target_condition_list,
>>         build_target_command_list): Removed.
>>         (build_target_condition_list_first, build_target_command_list_first,
>>         build_target_command_list_and_condition_list): New.
>>         (insert_bp_location): Call
>>         build_target_command_list_and_condition_list.
>>         * remote.c (remote_add_target_side_condition): Handle NULL.
>>         (remote_add_target_side_commands): Ditto.
>>
>> 2013-04-27  Hui Zhu  <hui@codesourcery.com>
>>
>>         PR gdb/15180
>>         * linux-low.c (linux_wait_1): Update arguments of function
>>         gdb_condition_true_at_breakpoint.
>>         (need_step_over_p): Ditto.
>>         * mem-break.c (add_condition_to_breakpoint): Add new condition
>>         to tail.
>>         (gdb_condition_true_at_breakpoint): Execute commands.
>>         (add_commands_to_breakpoint): Add new commands to tail.
>>         * mem-break.h (gdb_condition_true_at_breakpoint): Update arguments.

Attachment: dprintf-remote-cond.txt
Description: Text document

Attachment: dprintf-remote-cond-server.txt
Description: Text document