From patchwork Wed Sep 2 01:40:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 8550 Received: (qmail 61612 invoked by alias); 2 Sep 2015 01:40:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 61585 invoked by uid 89); 2 Sep 2015 01:40:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_20, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-pa0-f73.google.com Received: from mail-pa0-f73.google.com (HELO mail-pa0-f73.google.com) (209.85.220.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 02 Sep 2015 01:40:20 +0000 Received: by pacii12 with SMTP id ii12so1314598pac.1 for ; Tue, 01 Sep 2015 18:40:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to :content-type; bh=yD8USNNHpYuBCfOPBMlUOqmVy5Z6KvYwFbojNcvbwZg=; b=YfTv3l0lsCo7oGbYZzN2ww2iwbrwt47BWVpoPZpdmCLhfZaplkht4qjPkxolusobUo Dq258vwsuixABRIXUalTAACJgxLovIsmjgP+REUQozY723S261VVMcP0cY/0G8eIr/ho Mp1MCCi8Ig19bD40ytb6DAg7jZz/Of4M5hyJMnE4BXH4Qd9IXeeRW+sKYoVWjV/NXA6V FqmqZUTpCthNEqZ1DW8i87+0oG5CwBpY4AkjHDDXyzOA3HAFI0juXoVeKt2TczM6H+Z4 mkQ66bXAtl3Hrsm1r0wk9oFYDO9UIcjauneC65aOR+3SPVxHMPLwg+LoRpc91fatNxww j6jw== X-Gm-Message-State: ALoCoQmh0S0+YP1fhKmaV+LBLpJNRmvDvfRhlqBZCUmnJkh46s8tT4iO+LpZHyx5JjsBLoZyhnQjHoL2S0HvFTaHw8rpECc1nK8ZKOKPQ/CzOhFIjK1Um4Un0sPeTcoa8A0bnpDllRiW9PvOMFp5zunIk/63OUIxZ7KES+giyFp/HM9m8WvOR0I= MIME-Version: 1.0 X-Received: by 10.66.102.6 with SMTP id fk6mr33803565pab.6.1441158018598; Tue, 01 Sep 2015 18:40:18 -0700 (PDT) Message-ID: <047d7bd9037cc3fa92051eb9bf4a@google.com> Date: Wed, 02 Sep 2015 01:40:18 +0000 Subject: [PATCH] Don't look up agent symbols if it's off From: Doug Evans To: gdb-patches@sourceware.org X-IsSubscribed: yes 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 * 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)