* [PATCH v2] Fix argument passing in mi_run_cmd_full @ 2014-05-13 21:47 Simon Marchi 2014-05-15 18:40 ` Tom Tromey 2014-05-19 19:39 ` Tom Tromey 0 siblings, 2 replies; 7+ messages in thread From: Simon Marchi @ 2014-05-13 21:47 UTC (permalink / raw) To: gdb-patches; +Cc: Simon Marchi Again, this patch was accepted almost a year ago, I want to make sure that it is still OK. Original submission: https://sourceware.org/ml/gdb-patches/2013-06/msg00739.html Passing arguments did not work when use_mi_command was set. gdb/testsuite/ChangeLog: 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling "-exec-arguments" or "set args" before running the inferior. --- gdb/testsuite/lib/mi-support.exp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index ad58775..b27b268 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -818,9 +818,11 @@ proc mi_run_cmd_full {use_mi_command args} { if {$use_mi_command} { set run_prefix "220-exec-" set run_match "220" + set set_args_cmd "-exec-arguments" } else { set run_prefix "" set run_match "" + set set_args_cmd "set args" } if [target_info exists gdb_init_command] { @@ -862,7 +864,16 @@ proc mi_run_cmd_full {use_mi_command args} { return 0 } - send_gdb "${run_prefix}run $args\n" + send_gdb "${set_args_cmd} $args\n" + gdb_expect { + -re "$mi_gdb_prompt$" { } + default { + perror "settings args failed" + return -1 + } + } + + send_gdb "${run_prefix}run\n" gdb_expect { -re "${run_match}\\^running\r\n(\\*running,thread-id=\"\[^\"\]+\"\r\n|=thread-created,id=\"1\",group-id=\"\[0-9\]+\"\r\n)*(${library_loaded_re})*(${thread_selected_re})?${mi_gdb_prompt}" { } -- 2.0.0.rc0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-13 21:47 [PATCH v2] Fix argument passing in mi_run_cmd_full Simon Marchi @ 2014-05-15 18:40 ` Tom Tromey 2014-05-15 19:47 ` Simon Marchi 2014-05-19 19:39 ` Tom Tromey 1 sibling, 1 reply; 7+ messages in thread From: Tom Tromey @ 2014-05-15 18:40 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: Simon> Again, this patch was accepted almost a year ago, I want to make sure Simon> that it is still OK. Simon> Original submission: Simon> https://sourceware.org/ml/gdb-patches/2013-06/msg00739.html Simon> 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> Simon> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling Simon> "-exec-arguments" or "set args" before running the inferior. Yes, still ok, thank you. Tom ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-15 18:40 ` Tom Tromey @ 2014-05-15 19:47 ` Simon Marchi 0 siblings, 0 replies; 7+ messages in thread From: Simon Marchi @ 2014-05-15 19:47 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 14-05-15 02:40 PM, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: > > Simon> Again, this patch was accepted almost a year ago, I want to make sure > Simon> that it is still OK. > > Simon> Original submission: > Simon> https://sourceware.org/ml/gdb-patches/2013-06/msg00739.html > > Simon> 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> > > Simon> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling > Simon> "-exec-arguments" or "set args" before running the inferior. > > Yes, still ok, thank you. > > Tom > Pushed, thanks! ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-13 21:47 [PATCH v2] Fix argument passing in mi_run_cmd_full Simon Marchi 2014-05-15 18:40 ` Tom Tromey @ 2014-05-19 19:39 ` Tom Tromey 2014-05-20 16:40 ` Simon Marchi 1 sibling, 1 reply; 7+ messages in thread From: Tom Tromey @ 2014-05-19 19:39 UTC (permalink / raw) To: Simon Marchi; +Cc: gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: Simon> 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> Simon> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling Simon> "-exec-arguments" or "set args" before running the inferior. This patch causes a test failure here: Running ../../../binutils-gdb/gdb/testsuite/gdb.mi/mi-start.exp ... FAIL: gdb.mi/mi-start.exp: run to main (unknown output after running) This failure isn't present in the previous revision. I've appended the relevant section of gdb.log in case that is helpful. Tom (gdb) 220-exec-run =thread-group-started,id="i1",pid="21201" =thread-created,id="1",group-id="i1" 220^running *running,thread-id="all" (gdb) mi_expect_stop: expecting: \*stopped,reason="breakpoint-hit",disp="del",bkptno=" [0-9]+",frame={addr="0x[0-9A-Fa-f]+",func="main",args=\[\],(?:file="[^ ]*.*mi-start.c",fullname="(/[^\n]*/|\\\\[^\\]+\\[^\n]+\\|\\[^\\][^\n]*\\|[a-zA-Z ]:[^\n]*\\).*mi-start.c",line="[0-9]+"|from=".*mi-start.c")},thread-id="[0-9]+", stopped-threads=[^ ]* (=thread-selected,id="[0-9]+" |=(?:breakpoint-created|breakpoint-deleted)[^ ]+" )*[(]gdb[)] $ =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x8 6-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-gro up="i1" =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name=" /lib64/libm.so.6",symbols-loaded="0",thread-group="i1" =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name=" /lib64/libc.so.6",symbols-loaded="0",thread-group="i1" =thread-exited,id="1",group-id="i1" =thread-group-exited,id="i1",exit-code="0" *stopped,reason="exited-normally" (gdb) got =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linu x-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread -group="i1" =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name=" /lib64/libm.so.6",symbols-loaded="0",thread-group="i1" =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name=" /lib64/libc.so.6",symbols-loaded="0",thread-group="i1" =thread-exited,id="1",group-id="i1" =thread-group-exited,id="i1",exit-code="0" *stopped,reason="exited-normally" (gdb) FAIL: gdb.mi/mi-start.exp: run to main (unknown output after running) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-19 19:39 ` Tom Tromey @ 2014-05-20 16:40 ` Simon Marchi 2014-05-20 22:24 ` Simon Marchi 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2014-05-20 16:40 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Mon 19 May 2014 03:39:09 PM EDT, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: > > Simon> 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> > Simon> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling > Simon> "-exec-arguments" or "set args" before running the inferior. > > This patch causes a test failure here: Ehh, that's what I get for not re-running the testsuite, even after a year. I will send something soon about that. > Running ../../../binutils-gdb/gdb/testsuite/gdb.mi/mi-start.exp ... > FAIL: gdb.mi/mi-start.exp: run to main (unknown output after running) > > This failure isn't present in the previous revision. > > I've appended the relevant section of gdb.log in case that is helpful. > > Tom > > (gdb) > 220-exec-run > =thread-group-started,id="i1",pid="21201" > =thread-created,id="1",group-id="i1" > 220^running > *running,thread-id="all" > (gdb) > mi_expect_stop: expecting: \*stopped,reason="breakpoint-hit",disp="del",bkptno=" > [0-9]+",frame={addr="0x[0-9A-Fa-f]+",func="main",args=\[\],(?:file="[^ > ]*.*mi-start.c",fullname="(/[^\n]*/|\\\\[^\\]+\\[^\n]+\\|\\[^\\][^\n]*\\|[a-zA-Z > ]:[^\n]*\\).*mi-start.c",line="[0-9]+"|from=".*mi-start.c")},thread-id="[0-9]+", > stopped-threads=[^ > ]* > (=thread-selected,id="[0-9]+" > |=(?:breakpoint-created|breakpoint-deleted)[^ > ]+" > )*[(]gdb[)] > $ > =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linux-x8 > 6-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread-gro > up="i1" > =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name=" > /lib64/libm.so.6",symbols-loaded="0",thread-group="i1" > =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name=" > /lib64/libc.so.6",symbols-loaded="0",thread-group="i1" > =thread-exited,id="1",group-id="i1" > =thread-group-exited,id="i1",exit-code="0" > *stopped,reason="exited-normally" > (gdb) > got =library-loaded,id="/lib64/ld-linux-x86-64.so.2",target-name="/lib64/ld-linu > x-x86-64.so.2",host-name="/lib64/ld-linux-x86-64.so.2",symbols-loaded="0",thread > -group="i1" > =library-loaded,id="/lib64/libm.so.6",target-name="/lib64/libm.so.6",host-name=" > /lib64/libm.so.6",symbols-loaded="0",thread-group="i1" > =library-loaded,id="/lib64/libc.so.6",target-name="/lib64/libc.so.6",host-name=" > /lib64/libc.so.6",symbols-loaded="0",thread-group="i1" > =thread-exited,id="1",group-id="i1" > =thread-group-exited,id="i1",exit-code="0" > *stopped,reason="exited-normally" > (gdb) > > FAIL: gdb.mi/mi-start.exp: run to main (unknown output after running) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-20 16:40 ` Simon Marchi @ 2014-05-20 22:24 ` Simon Marchi 2014-05-21 8:46 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Simon Marchi @ 2014-05-20 22:24 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Tue 20 May 2014 12:40:01 PM EDT, Simon Marchi wrote: > On Mon 19 May 2014 03:39:09 PM EDT, Tom Tromey wrote: >>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes: >> >> Simon> 2014-05-13 Simon Marchi <simon.marchi@ericsson.com> >> Simon> * lib/mi-support.exp (mi_run_cmd_full): Set arguments by calling >> Simon> "-exec-arguments" or "set args" before running the inferior. >> >> This patch causes a test failure here: > > Ehh, that's what I get for not re-running the testsuite, even after a > year. I will send something soon about that. mi-start.exp passes "--start" to mi_run_cmd, which is intended to be an argument to -exec-run. My understanding was that the parameter "args" of mi_run_cmd_full was for arguments to pass to the test program (hence, my patch). A quick grep shows that mi-start.exp is the only place where arguments are actually passed to mi_run_cmd. At the time the patch was written, nowhere in the test suite were parameters passed to mi_run_cmd, so I just assumed that this was the intended behavior. I ended up not even using this fix, since Pedro suggested that to modify the behavior of a test program, I should stop at a breakpoint and modify the value of a global variable instead. Perhaps the original behavior was not bad after all: $args is passed to the command used to run the program, "run" or "-exec-run", depending on the value of $use_mi_command. Therefore, the meaning/semantic of the $args parameter changes based on the value of $use_mi_command: arguments passed to the test program if false, arguments to -exec-run (such as --start) if true. If you use MI and want to pass arguments to the test program, then you need to manually call -exec-arguments before (or a helper proc could be created for that). If documented properly, I think this way is good. If we decide to stay with the current behavior where $args always means the arguments to pass to the test program, then we'll need to add a separate parameter to mi_run_cmd_full, mi_run_cmd and mi_run_with_cli to convey the arguments to pass to the run command (to be able to pass the --start). This makes things a little bit messy: proc mi_run_cmd {args} will become proc mi_run_cmd {test_program_args run_command_args} which means that most calls to mi_run_cmd will have to be changed from mi_run_cmd to mi_run_cmd "" "" because of the special vararg-like meaning of the parameter named "args". In my opinion, I think the old behavior was fine, it just needs to be documented better (clarify what $args means in the mi_run_* procedures). I would like to hear what you think about this. Thanks, Simon ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix argument passing in mi_run_cmd_full 2014-05-20 22:24 ` Simon Marchi @ 2014-05-21 8:46 ` Pedro Alves 0 siblings, 0 replies; 7+ messages in thread From: Pedro Alves @ 2014-05-21 8:46 UTC (permalink / raw) To: Simon Marchi, Tom Tromey; +Cc: gdb-patches On 05/20/2014 11:24 PM, Simon Marchi wrote: > In my opinion, I think the old behavior was fine, it just needs to be documented better (clarify what $args means in the mi_run_* procedures). I agree, because it's also the simplest, and as you say, > I ended up not even using this fix, since Pedro suggested that to modify > the behavior of a test program, I should stop at a breakpoint and modify > the value of a global variable instead. nothing is actually making use of the original fix. If we end up needing to pass program arguments to mi_run_cmd in MI mode, we can revisit. Thanks, -- Pedro Alves ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-21 8:46 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-13 21:47 [PATCH v2] Fix argument passing in mi_run_cmd_full Simon Marchi 2014-05-15 18:40 ` Tom Tromey 2014-05-15 19:47 ` Simon Marchi 2014-05-19 19:39 ` Tom Tromey 2014-05-20 16:40 ` Simon Marchi 2014-05-20 22:24 ` Simon Marchi 2014-05-21 8:46 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox