[PATCH 1/2 v2] Re-check fastpoint after reloading symbols.

[Wei-cheng Wang <cole945 at gmail dot com>]

Hi,

I just realized that disabled tracepoints will still be downloaded
on the target by 'tstart' command, and they can be re-enabled (by
QTEnable packet) when trace is running, so I cannot simply disable
an invalid tracepoint to prevent invalid tracepoint being downloaded.

Scenario 1: the trace is not running.

Because all tracepoints will be downloaded, when 'tstart', it doesn't
matter whether they are disabled or not.  I think if any invalid
tracepoint is found, simply throw an error to force user to delete
invalid tracepoint.

Scenario 2: the trace is already running.

I think the check should be placed before update_global_location_list,
because it will try to download new tracepoints on target.
(download_tracepoint_locations).  update_breakpoint_locations will not
download disabled tracepoints, so we should disable invalid tracepoints
before calling update_global_location_list.
(see: should_be_inserted in download_tracepoint_locations)

Otherwise, if we don't disable invalid tracepoints, an error is thrown,
and the program will stop at _dl_debug_state.  Then the user has to delete
the tracepoint manually to continue the execution.

If a pending tracepoint is resovled after trace is running, and the user
tries to enable an invalid tracepoint after , server will report an
error that it is not installed.  It won't run into internal error.

Any suggestion?

Thanks,
Wei-cheng

--

gdb/ChangeLog

2015-08-23  Wei-cheng Wang  <cole945@gmail.com>

	* breakpoint.c (update_breakpoint_locations): Check
	fast tracepoints after reloading symbols.
	* remote.c (remote_download_tracepoint): Change
	internal_error to error.
	* pending.exp (pending_tracepoint_resolved,
	pending_tracepoint_resolved, pending_tracepoint_works,
	pending_tracepoint_resolved_during_trace,
	pending_tracepoint_installed_during_trace,
	pending_tracepoint_with_action_resolved): Fix test cases accordingly.

---
 gdb/breakpoint.c                    | 22 +++++++++++++
 gdb/remote.c                        |  4 +--
 gdb/testsuite/gdb.trace/pending.exp | 62 +++++++++++++++++++++++++++++++++++--
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 052aeb9..7645d18 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14148,6 +14148,28 @@ update_breakpoint_locations (struct breakpoint *b,
   if (!locations_are_equal (existing_locations, b->loc))
     observer_notify_breakpoint_modified (b);
 
+  /* Check fast tracepoints after symbols have been re-loaded.
+     For example, a pending tracepoint just becomes available after
+     a new shared object being loaded.  We didn't check it before,
+     because we have no idea where it is.  */
+  if (b->type == bp_fast_tracepoint)
+    {
+      TRY
+	{
+	  check_fast_tracepoint_sals (b->gdbarch, &sals);
+	}
+      CATCH (e, RETURN_MASK_ERROR)
+	{
+	  /* If that tracepoint cannot be inserted, disable it,
+	     so update_global_location_list will not install it
+	     to the target.  */
+	  b->enable_state = bp_disabled;
+	  exception_fprintf (gdb_stderr, e, _("Invalid tracepoint %d: "),
+			     b->number);
+	}
+      END_CATCH
+    }
+
   update_global_location_list (UGLL_MAY_INSERT);
 }
 
diff --git a/gdb/remote.c b/gdb/remote.c
index ca1f0df..2e514fb 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11145,9 +11145,7 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
 	  else
 	    /* If it passed validation at definition but fails now,
 	       something is very wrong.  */
-	    internal_error (__FILE__, __LINE__,
-			    _("Fast tracepoint not "
-			      "valid during download"));
+	    error (_("Fast tracepoint not valid during download"));
 	}
       else
 	/* Fast tracepoints are functionally identical to regular
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index ed36cac..94f39ac 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -65,6 +65,7 @@ proc pending_tracepoint_resolved { trace_type } {
 	global binfile
 	global srcfile
 	global lib_sl1
+	global gdb_prompt
 
 	# Start with a fresh gdb.
 	gdb_exit
@@ -89,12 +90,22 @@ proc pending_tracepoint_resolved { trace_type } {
 	    "breakpoint function"
 
 	gdb_run_cmd
-	gdb_test "" "Breakpoint 2, main.*"
+	set test "Breakpoint 2, main"
+	set yesno "y"
+	gdb_test_multiple "" $test {
+	    -re ".*May not have a fast tracepoint at.*$test.*" {
+		set yesno "n"
+		pass "$test (invalid)"
+	    }
+	    -re "Breakpoint.*main.*at.*$gdb_prompt $" {
+		pass $test
+	    }
+	}
 
 	# Run to main which should resolve a pending tracepoint
 	gdb_test "info trace" \
 	    "Num     Type\[ \]+Disp Enb Address\[ \]+What.*
-\[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \]+keep y.*pendfunc.*" \
+\[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \]+keep $yesno.*pendfunc.*" \
 	    "single tracepoint info"
     }
 }
@@ -151,7 +162,16 @@ proc pending_tracepoint_works { trace_type } {
 		    fail $test
 		}
 	    }
-
+	    -re "No tracepoints enabled.*$gdb_prompt $" {
+		if [string equal $trace_type "ftrace"] {
+		    # There is no valid tracepoint enabled.
+		    pass "$test (no tracepoints)"
+		    # Skip the rest of the tests.
+		    return
+		} else {
+		    fail $test
+		}
+	    }
 	}
 
 	gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*" \
@@ -221,6 +241,18 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
 		fail $test
 	    }
 	}
+	-re "May not have a fast tracepoint at.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., the location is invalid for the
+		# tracepoint).
+		pass "$test (invalid)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass $test
 	}
@@ -294,6 +326,18 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 		fail $test
 	    }
 	}
+	-re "May not have a fast tracepoint at.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., the location is invalid for the
+		# tracepoint).
+		pass "$test (invalid)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
            pass $test
        }
@@ -475,6 +519,18 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
 		fail $test
             }
 	}
+	-re "May not have a fast tracepoint at.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., the location is invalid for the
+		# tracepoint).
+		pass "$test (invalid)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass "continue to marker 2"
 	}
-- 
1.9.1