[PATCH] Don't look up agent symbols if it's off

[Doug Evans <dje at google dot com>]

Hi.

Looking at some agent code I see there's a bit of TLC needed here.
[E.g., target_can_use_agent isn't used anywhere.]

I didn't want to get bogged down in fixing all of it,
but I do want to address one issue:

According to common/agent.c there are three symbols that are needed:

  IPA_SYM(helper_thread_id),
  IPA_SYM(cmd_buf),
  IPA_SYM(capability),

but the default value of "set agent" is "off".

(gdb) help set agent
Set debugger's willingness to use agent as a helper.
If on, GDB will delegate some of the debugging operations to the
agent, if the target supports it.  This will speed up those
operations that are supported by the agent.
If off, GDB will not use agent, even if such is supported by the
target.

Why are we looking up these symbols if the agent is off?
It's possible I'm missing something.
I can imagine that "agent" is ambiguous here as I see
linux-nat.c:linux_child_static_tracepoint_markers_by_strid
calling agent_run_command but I can't find a test that first checks
that that's ok (e.g., what if "set agent" is "off"?).

This patch stops the symbol lookup if the agent is off,
and adds code to look them up when the agent is turned on.
But I think more is needed here.
E.g, what happens if linux_child_static_tracepoint_markers_by_strid
gets called but we haven't found the address of
gdb_agent_cmd_buf?
Perhaps that "can't happen", but where does one look to prove that?

I ran the testsuite with --target_board=native-stdio-gdbserver,
but having dug a bit deeper I'm not sure it was useful.
The default for "set agent" is off and I can't
find any test that actually turns it on. So now I'm wondering
if some TLC needs to be applied to the testsuite too. :-(
E.g., does gdb.trace/ftrace.exp actually test what it claims to test?
I added a "set agent on" to ftrace.exp and got identical results,
but IWBN to have a way to verify the intended code got exercised.
E.g., I see tests for use_agent in gdbserver/tracepoint.c,
but I also see fallbacks in case the test fails.
Why aren't we testing with "set agent on"?

I also see pr 13808 is still with us.
	setup_kfail "gdb/13808" "x86_64-*-linux*"
	gdb_test "print globvar" " = 1"

btw, where do I get ust/marker.h?
It's needed by some tests but the packages that seem like they should
have it don't (e.g., lttng-ust{,-devel}).

2015-09-01  Doug Evans  <dje@google.com>

	* agent.c (set_can_use_agent): If agent has been turned on, lookup
	agent symbols if necessary.
	(agent_new_objfile): Don't look up agent symbols if agent is off.
	* common/agent.h (agent_look_up_symbols): Improve comment.

diff --git a/gdb/agent.c b/gdb/agent.c
index a5f6cdc..57f993d 100644
--- a/gdb/agent.c
+++ b/gdb/agent.c
@@ -47,8 +47,14 @@ static void
 set_can_use_agent (char *args, int from_tty, struct cmd_list_element *c)
 {
   if (target_use_agent (can_use_agent == can_use_agent_on) == 0)
-    /* Something wrong during setting, set flag to default value.  */
+    /* Something wrong during setting, agent is turned off.  */
     can_use_agent = can_use_agent_off;
+
+  /* If we turned it on, and the support symbols haven't been loaded yet,
+     do it now.  */
+  if (can_use_agent == can_use_agent_on
+      && !agent_loaded_p ())
+    agent_look_up_symbols (NULL);
 }

 /* -Wmissing-prototypes */
@@ -60,7 +66,9 @@ extern initialize_file_ftype _initialize_agent;
 static void
 agent_new_objfile (struct objfile *objfile)
 {
-  if (objfile == NULL || agent_loaded_p ())
+  if (can_use_agent == can_use_agent_off
+      || objfile == NULL
+      || agent_loaded_p ())
     return;

   agent_look_up_symbols (objfile);
diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 5bb18641..2e780e0 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -80,7 +80,9 @@ agent_loaded_p (void)
 }

 /* Look up all symbols needed by agent.  Return 0 if all the symbols are
-   found, return non-zero otherwise.  */
+   found, return non-zero otherwise.
+   ARG is either an objfile that was just loaded, or NULL meaning to search
+   all objfiles.  */

 int
 agent_look_up_symbols (void *arg)