Message ID | 533AABE1.8040101@redhat.com |
---|---|
State | Superseded |
Headers | show |
On 04/01/2014 08:06 PM, Pedro Alves wrote: > As part of that project, we'll definitely need to move the > whole remote_protocol_packets global array to remote_state. > This global array already exists. I've just moved a few more manually > managed flags into this array. Note this is listed as a TODO in > the MultiTarget project's wiki page. > Therefore, I'm not really adding any conflict, as that work will > already have to be done. When that is done, we'll likely end up > with passing a 'struct remote_state *' pointer to the new packet_support > function. And with that in mind, it just looks like unnecessary > churn to remove the parameter from remote_multi_process_p now (and > update all callers), only to add it back again. Do you agree? > Yes, that is fine to keep unused parameter 'rs' remote_multi_process_p. >> > Not sure it is part >> > of your plan to convert the global state to per-target state. > I don't plan to work on it myself in the near future, but it's > definitely the way to go. > OK, got it. >>> >> + >>> >> + # Force-disable the InstallInTrace RSP feature. >>> >> + gdb_test_no_output "set remote install-in-trace-packet off" >>> >> + >>> >> + # Set a tracepoint while a trace experiment is ongoing. >>> >> + set test "set tracepoint on set_tracepoint" >>> >> + gdb_test_multiple "${trace_type} set_tracepoint" $test { >>> >> + -re "Target returns error code .* too far .*$gdb_prompt $" { >>> >> + if [string equal $trace_type "ftrace"] { >>> >> + # The target was unable to install the fast tracepoint >>> >> + # (e.g., jump pad too far from tracepoint). >>> >> + pass "$test (too far)" >> > >> > A nit here, a fast tracepoint is not installed due to "jump pad too far" >> > instead of "set remote install-in-trace-packet off". Do we want to emit >> > a PASS here? > Oh, hmm. Copy-paste from the other function in the file, and I > just didn't think about that part much. It actually makes no sense > to even expect that the target sends back any error code. We've just > disabled installing tracepoints while a trace experiment is running, > so we shouldn't see any response from the target at all. > >> > IMO, UNSUPPORTED is better. > Agreed, though note you made this a PASS in tracepoint_change_loc_1. ;-) > Sorry about that :) patch v2 looks good to me.
On 04/01/2014 01:46 PM, Yao Qi wrote:
> patch v2 looks good to me.
Thanks Yao. I've pushed this in.
diff --git a/gdb/remote.c b/gdb/remote.c index 1aac5e3..dcd3cdd 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1432,6 +1432,7 @@ show_remote_protocol_Z_packet_cmd (struct ui_file *file, int from_tty, } /* Returns true if the multi-process extensions are in effect. */ + static int remote_multi_process_p (struct remote_state *rs) { diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp index ff6918a..d1a7aee 100644 --- a/gdb/testsuite/gdb.trace/change-loc.exp +++ b/gdb/testsuite/gdb.trace/change-loc.exp @@ -286,7 +286,7 @@ proc tracepoint_change_loc_2 { trace_type } { # doesn't work when we force-disable the InstallInTrace RSP feature. proc tracepoint_install_in_trace_disabled { trace_type } { - with_test_prefix "3 $trace_type" { + with_test_prefix "InstallInTrace disabled: $trace_type" { global testfile global srcfile global pcreg @@ -312,27 +312,18 @@ proc tracepoint_install_in_trace_disabled { trace_type } { "Tracepoint \[0-9\] at.* file .*$srcfile, line.*" \ "set tracepoint on main" + gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \ + "breakpoint on marker" + gdb_test_no_output "tstart" # Force-disable the InstallInTrace RSP feature. gdb_test_no_output "set remote install-in-trace-packet off" # Set a tracepoint while a trace experiment is ongoing. - set test "set tracepoint on set_tracepoint" - gdb_test_multiple "${trace_type} set_tracepoint" $test { - -re "Target returns error code .* too far .*$gdb_prompt $" { - if [string equal $trace_type "ftrace"] { - # The target was unable to install the fast tracepoint - # (e.g., jump pad too far from tracepoint). - pass "$test (too far)" - } else { - fail $test - } - } - -re "\r\n$gdb_prompt $" { - pass $test - } - } + gdb_test "${trace_type} set_tracepoint" \ + "racepoint .* at .* set_tracepoint.*" \ + "set tracepoint on set_tracepoint" gdb_trace_setactions "set action for tracepoint" "" \ "collect \$$pcreg" "^$" @@ -340,8 +331,16 @@ proc tracepoint_install_in_trace_disabled { trace_type } { # Make sure the tracepoint is _not_ installed on the target. gdb_test "info trace" \ "Num Type\[ \]+Disp Enb Address\[ \]+What.* -\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*3\.1.* in func4.*not installed on target.*3\.2.* in func4.*not installed on target.*" \ - "tracepoint with two locations" +\[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*installed on target.*\<MULTIPLE\>.*4\.1.* in func4.*not installed on target.*4\.2.* in func4.*not installed on target.*" \ + "tracepoint is not installed" + + gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \ + "continue to marker" + + gdb_test_no_output "tstop" + + # Nothing should have been collected. + gdb_test "tfind" "Target failed to find requested trace frame\\..*" } }