[1/2,v2] Re-check fastpoint after reloading symbols.
Commit Message
"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> However, if just a single tracepoint is invalid, we simply get an
> internal error here, which is the problem Wei-cheng is trying to fix.
> [ The same gdbarch_fast_tracepoint_valid_at call is done at the time
> the tracepoint was installed originally. However, if at that time
> the tracepoint was pending, and its location has now been resolved,
> that check was not re-done. ]
Thanks for the clarification, Ulrich.
>
> I see two issues here:
>
> - I agree with you that there's duplicated checks here. The gdbarch
> callback gdbarch_fast_tracepoint_valid_at is apparently supposed to
> duplicate the logic done in gdbserver. In any case, any tracepoint
> considered "valid" by callback is expected to succeed on the target.
>
> - The ICE can be triggered by normal user action (if the result of
> gdbarch_fast_tracepoint_valid_at changes if locations are recomputed,
> see above).
>
> What I had asked Wei-cheng to implement is to fix these two issues:
> properly duplicate the validity check for PowerPC, and re-validate
> every time locations are resolved.
That is fine by me in general, and I attach a reproducer to trigger GDB
internal error on x86. Wei-cheng's patch fixes this internal error, and
instead, a normal error is emitted.
>
> Maybe it would indeed be better to move the validity-checking logic
> completely to the target side, i.e. always attempt to download the
> tracepoint, and react more intelligently if that fails (e.g. downgrade
> to a regular tracepoint?). I'm not sure if that is doable with the
> current remote protocol definition. Thoughts?
The duplicated checks in both GDB and GDBserver sides aren't that bad to
me, however, I am worried about using internal knowledge of
GDBserver and IPA to validate fast tracepoint in GDB side. I raised
this here https://sourceware.org/ml/gdb-patches/2015-09/msg00041.html
I suspect that we won't see GDB internal error on PPC if we don't do
checks using GDBserver internal knowledge. Without GDBserver internal
knowledge, ppc_fast_tracepoint_valid_at should always return true.
@@ -492,6 +492,64 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
gdb_test "tfind" "Target failed to find requested trace frame..*" "tfind test frame"
}}
+# Verify setting tracepoint on a very short instruction.
+
+proc pending_tracepoint_on_short_insn { trace_type } \
+{ with_test_prefix "$trace_type on short insn" \
+{
+ if { ![istarget "x86_64-*"] && ![istarget "i?86-*"] } {
+ return
+ }
+
+ global executable
+ global srcfile
+ global lib_sl1
+ global gdb_prompt
+
+ # Start with a fresh gdb.
+ clean_restart $executable
+ if ![runto_main] {
+ fail "Can't run to main"
+ return -1
+ }
+
+ gdb_test_multiple "$trace_type set_point3" "set pending tracepoint on set_point2" {
+ -re ".*Make \(fast |\)tracepoint pending.*y or \\\[n\\\]. $" {
+ gdb_test "y" "\(Fast t|T\)racepoint.*set_point3.*pending." \
+ "set pending tracepoint (without symbols)"
+ }
+ }
+
+ gdb_test "info trace" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \]+keep y.*PENDING.*set_point3.*" \
+ "single pending tracepoint on set_point3"
+
+ gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
+ "breakpoint on marker"
+
+ gdb_test_no_output "tstart" "start trace experiment"
+
+ gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
+ "continue to marker 1"
+
+ set test "continue to marker 2"
+ gdb_test_multiple "continue" $test {
+ -re "Continuing.\r\n(Reading .* from remote target...\r\n)?\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+ pass "continue to marker 2"
+ }
+
+ }
+
+ gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
+
+ # tracepoint should be resolved.
+ gdb_test "info trace" \
+ "Num Type\[ \]+Disp Enb Address\[ \]+What.*
+\[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \]+keep y.*set_point3.*" \
+ "tracepoint is resolved"
+}}
+
pending_tracepoint_resolved "trace"
pending_tracepoint_works "trace"
@@ -506,6 +564,8 @@ pending_tracepoint_with_action_resolved "trace"
pending_tracepoint_installed_during_trace "trace"
+pending_tracepoint_on_short_insn "trace"
+
# Re-compile test case with IPA.
set libipa [get_in_proc_agent]
gdb_load_shlibs $libipa
@@ -524,3 +584,4 @@ pending_tracepoint_disconnect_during_trace "ftrace"
pending_tracepoint_disconnect_after_resolved "ftrace"
pending_tracepoint_with_action_resolved "ftrace"
pending_tracepoint_installed_during_trace "ftrace"
+pending_tracepoint_on_short_insn "ftrace"
@@ -37,4 +37,19 @@ pendfunc2 (int x)
" call " SYMBOL(foo) "\n"
#endif
);
+
+ /* `set_point3' is the label where we'll set multiple tracepoints at,
+ but the instruction at the label isn't large enough to fit a
+ fast tracepoint jump. */
+ asm (" .global " SYMBOL(set_point3) "\n"
+ SYMBOL(set_point3) ":\n"
+#if defined __i386__
+ "push %ecx\n"
+ "pop %ecx\n"
+#elif defined __x86_64__
+ "pushq %rax\n"
+ "popq %rax\n"
+#else
+#endif
+ );
}