From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22662 invoked by alias); 6 Aug 2014 17:49:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 22622 invoked by uid 89); 6 Aug 2014 17:49:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ob0-f202.google.com Received: from mail-ob0-f202.google.com (HELO mail-ob0-f202.google.com) (209.85.214.202) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 06 Aug 2014 17:49:11 +0000 Received: by mail-ob0-f202.google.com with SMTP id wp18so536539obc.3 for ; Wed, 06 Aug 2014 10:49:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=tQHeP5sEW6jy7TgJW9xRRjY/8CB+EsNmdKsVHMVrqSA=; b=E+yEKF4ymn9b4fY2nwRt7nEIJL7axbw+26pQ+YqGX3By5GH+1KSX8oU29qQidlL+Wm SRlrmMKp+Fibzd6ZFFL18D4V5sZbnH109PPJtx+fDhGxM6F0t3BQtuVpx3uqyUeuEfKR 8H1scAOHfrR5/TtFOp9SPqppJQbp2tqoAaBVWvANCWv4iHsVXPgN8Ebux9NBW6ZgOzpQ 2GHpAvNOXfD3KpDbaVhlaXd1ESDEfc/R9o/15xvyaGDo1ycZtjtPGJ64XaXojKQnaIah MvMPL6KkmTMkhAjpQmMtXLeywT/Qynz7cyfrJcQb+1kca6G7cgbfxtd6X+t1diVHUECl vNFA== X-Gm-Message-State: ALoCoQl691rxcftyZu9oq+zQewKozqgpdF8KBU2+WHk4a8ZdhJPRng4u3ZEYbyQB7Di176cg9bQx X-Received: by 10.182.251.135 with SMTP id zk7mr6519015obc.14.1407347349448; Wed, 06 Aug 2014 10:49:09 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id j43si109047yhh.5.2014.08.06.10.49.09 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 06 Aug 2014 10:49:09 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 9733031C480; Wed, 6 Aug 2014 10:49:08 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21474.27284.80140.944680@ruffy.mtv.corp.google.com> Date: Wed, 06 Aug 2014 17:49:00 -0000 To: Gary Benson Cc: gdb-patches@sourceware.org, Pedro Alves , Tom Tromey Subject: Re: [PATCH 05/11 v5] Add target/target.h In-Reply-To: <1406888377-25795-6-git-send-email-gbenson@redhat.com> References: <1406888377-25795-1-git-send-email-gbenson@redhat.com> <1406888377-25795-6-git-send-email-gbenson@redhat.com> X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00111.txt.bz2 Gary Benson writes: > This adds target/target.h. This file declares some functions that > the shared code can use and that the clients must implement. It > also changes some shared code to use these functions. > > gdb/ > 2014-08-01 Tom Tromey > Gary Benson > > * target/target.h: New file. > * Makefile.in (HFILES_NO_SRCDIR): Add target/target.h. > * target.h: Include target/target.h. > (target_resume, target_wait, target_stop, target_read_memory) > (target_write_memory, non_stop): Don't declare. > * target.c (target_read_uint32): New function. > * common/agent.c: Include target/target.h. > [!GDBSERVER]: Don't include target.h or infrun.h. > (agent_get_helper_thread_id): Use target_read_uint32. > (agent_run_command): Always use target_resume, target_stop, > target_wait, target_write_memory, and target_read_memory. > (agent_capability_check): Use target_read_uint32. > > gdb/gdbserver/ > 2014-08-01 Tom Tromey > Gary Benson > > * target.c (target_wait, target_stop, target_resume) > (target_read_memory, target_read_uint32) > (target_write_memory): New functions. > --- > gdb/ChangeLog | 16 ++++++++ > gdb/Makefile.in | 2 +- > gdb/common/agent.c | 76 +++--------------------------------- > gdb/gdbserver/ChangeLog | 7 +++ > gdb/gdbserver/target.c | 58 ++++++++++++++++++++++++++++ > gdb/target.c | 16 ++++++++ > gdb/target.h | 38 +------------------ > gdb/target/target.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++ > 8 files changed, 204 insertions(+), 107 deletions(-) > create mode 100644 gdb/target/target.h > > diff --git a/gdb/Makefile.in b/gdb/Makefile.in > index 8429ebc..090c912 100644 > --- a/gdb/Makefile.in > +++ b/gdb/Makefile.in > @@ -936,7 +936,7 @@ ctf.h nat/i386-cpuid.h nat/i386-gcc-cpuid.h target/resume.h \ > target/wait.h target/waitstatus.h nat/linux-nat.h nat/linux-waitpid.h \ > common/print-utils.h common/rsp-low.h nat/i386-dregs.h x86-linux-nat.h \ > i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \ > -common/common-debug.h > +common/common-debug.h target/target.h > > # Header files that already have srcdir in them, or which are in objdir. > > diff --git a/gdb/common/agent.c b/gdb/common/agent.c > index 7071ce6..5c19454 100644 > --- a/gdb/common/agent.c > +++ b/gdb/common/agent.c > @@ -21,10 +21,9 @@ > #include "server.h" > #else > #include "defs.h" > -#include "target.h" > -#include "infrun.h" > #include "objfiles.h" > #endif > +#include "target/target.h" > #include > #include "agent.h" > #include "filestuff.h" > @@ -126,23 +125,9 @@ agent_get_helper_thread_id (void) > { > if (helper_thread_id == 0) > { > -#ifdef GDBSERVER > - if (read_inferior_memory (ipa_sym_addrs.addr_helper_thread_id, > - (unsigned char *) &helper_thread_id, > - sizeof helper_thread_id)) > -#else > - enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ()); > - gdb_byte buf[4]; > - > - if (target_read_memory (ipa_sym_addrs.addr_helper_thread_id, > - buf, sizeof buf) == 0) > - helper_thread_id = extract_unsigned_integer (buf, sizeof buf, > - byte_order); > - else > -#endif > - { > - warning (_("Error reading helper thread's id in lib")); > - } > + if (target_read_uint32 (ipa_sym_addrs.addr_helper_thread_id, > + &helper_thread_id)) > + warning (_("Error reading helper thread's id in lib")); > } > > return helper_thread_id; > @@ -220,13 +205,8 @@ agent_run_command (int pid, const char *cmd, int len) > int tid = agent_get_helper_thread_id (); > ptid_t ptid = ptid_build (pid, tid, 0); > > -#ifdef GDBSERVER > - int ret = write_inferior_memory (ipa_sym_addrs.addr_cmd_buf, > - (const unsigned char *) cmd, len); > -#else > int ret = target_write_memory (ipa_sym_addrs.addr_cmd_buf, > (gdb_byte *) cmd, len); > -#endif > > if (ret != 0) > { > @@ -237,18 +217,7 @@ agent_run_command (int pid, const char *cmd, int len) > DEBUG_AGENT ("agent: resumed helper thread\n"); > > /* Resume helper thread. */ > -#ifdef GDBSERVER > -{ > - struct thread_resume resume_info; > - > - resume_info.thread = ptid; > - resume_info.kind = resume_continue; > - resume_info.sig = GDB_SIGNAL_0; > - (*the_target->resume) (&resume_info, 1); > -} > -#else > - target_resume (ptid, 0, GDB_SIGNAL_0); > -#endif > + target_resume (ptid, 0, GDB_SIGNAL_0); > > fd = gdb_connect_sync_socket (pid); > if (fd >= 0) > @@ -284,37 +253,18 @@ agent_run_command (int pid, const char *cmd, int len) > int was_non_stop = non_stop; > /* Stop thread PTID. */ > DEBUG_AGENT ("agent: stop helper thread\n"); > -#ifdef GDBSERVER > - { > - struct thread_resume resume_info; > - > - resume_info.thread = ptid; > - resume_info.kind = resume_stop; > - resume_info.sig = GDB_SIGNAL_0; > - (*the_target->resume) (&resume_info, 1); I certainly welcome replacing that with target_stop(). :-) > - } > - > - non_stop = 1; > - mywait (ptid, &status, 0, 0); The old gdbserver code set non_stop = 1 *after* asking the target to stop, whereas now it'll be done before (right?). Just checking that that's ok. E.g., I see a test for non_stop in linux_resume (which feels weird to be using in this context given that we're talking about target_stop :-)). > -#else > non_stop = 1; > target_stop (ptid); > > memset (&status, 0, sizeof (status)); > target_wait (ptid, &status, 0); > -#endif > non_stop = was_non_stop; > } > > if (fd >= 0) > { > -#ifdef GDBSERVER > - if (read_inferior_memory (ipa_sym_addrs.addr_cmd_buf, > - (unsigned char *) cmd, IPA_CMD_BUF_SIZE)) > -#else > if (target_read_memory (ipa_sym_addrs.addr_cmd_buf, (gdb_byte *) cmd, > IPA_CMD_BUF_SIZE)) > -#endif > { > warning (_("Error reading command response")); > return -1; > @@ -334,20 +284,8 @@ agent_capability_check (enum agent_capa agent_capa) > { > if (agent_capability == 0) > { > -#ifdef GDBSERVER > - if (read_inferior_memory (ipa_sym_addrs.addr_capability, > - (unsigned char *) &agent_capability, > - sizeof agent_capability)) > -#else > - enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ()); > - gdb_byte buf[4]; > - > - if (target_read_memory (ipa_sym_addrs.addr_capability, > - buf, sizeof buf) == 0) > - agent_capability = extract_unsigned_integer (buf, sizeof buf, > - byte_order); > - else > -#endif > + if (target_read_uint32 (ipa_sym_addrs.addr_capability, > + &agent_capability)) > warning (_("Error reading capability of agent")); > } > return agent_capability & agent_capa; > [...] LGTM with the above question resolved.