From patchwork Fri Jul 24 13:08:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 7834 Received: (qmail 37053 invoked by alias); 24 Jul 2015 13:08:28 -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 37037 invoked by uid 89); 24 Jul 2015 13:08:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f176.google.com Received: from mail-pd0-f176.google.com (HELO mail-pd0-f176.google.com) (209.85.192.176) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 24 Jul 2015 13:08:20 +0000 Received: by pdrg1 with SMTP id g1so13708481pdr.2 for ; Fri, 24 Jul 2015 06:08:19 -0700 (PDT) X-Received: by 10.66.221.226 with SMTP id qh2mr31542558pac.64.1437743298886; Fri, 24 Jul 2015 06:08:18 -0700 (PDT) Received: from E107787-LIN (gcc1-power7.osuosl.org. [140.211.15.137]) by smtp.gmail.com with ESMTPSA id f3sm4872006pat.31.2015.07.24.06.08.16 (version=TLS1_2 cipher=AES128-SHA256 bits=128/128); Fri, 24 Jul 2015 06:08:18 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH 7/8] Initialise target descrption after skipping extra traps for --wrapper References: <1437392126-29503-1-git-send-email-yao.qi@linaro.org> <1437392126-29503-8-git-send-email-yao.qi@linaro.org> <55B17806.2060000@redhat.com> <86bnf1hksj.fsf@gmail.com> <55B22711.4080307@redhat.com> Date: Fri, 24 Jul 2015 14:08:11 +0100 In-Reply-To: <55B22711.4080307@redhat.com> (Pedro Alves's message of "Fri, 24 Jul 2015 12:52:49 +0100") Message-ID: <86380dhfes.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Pedro Alves writes: > Yes, I think they should. It's what the GDB side used to do already > when that code was x86 gdb only (and hence that shell comment, which > gdbserver doesn't use yet), and then other archs followed suit. > "Going through the shell" is the exact same as going through > the exec wrapper -- we're not interested in debugging the shell, > and it may well have a different bitness/architecture of the target > program we want to debug. > > In practice that hook's implementation should want to avoid work if some > flag is not set, to avoid unnecessary ptrace syscalls and thus ends up > not doing anything when going through the shell/exec-wrapper. OK. > >> I'll test this patch on targets other than x86 (such as arm and >> aarch64) and see if it causes fails. I tested the patch on aarch64-linux, and no regressions are found. I'll push the following patch in. diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index fa9dc29..ac1ad6f 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -822,6 +822,14 @@ linux_create_inferior (char *program, char **allargs) return pid; } +/* Implement the arch_setup target_ops method. */ + +static void +linux_arch_setup (void) +{ + the_low_target.arch_setup (); +} + /* Attach to an inferior process. Returns 0 on success, ERRNO on error. */ @@ -2105,23 +2113,35 @@ linux_low_filter_event (int lwpid, int wstat) { struct process_info *proc; - /* Architecture-specific setup after inferior is running. This - needs to happen after we have attached to the inferior and it - is stopped for the first time, but before we access any - inferior registers. */ + /* Architecture-specific setup after inferior is running. */ proc = find_process_pid (pid_of (thread)); - if (proc->priv->new_inferior) + if (proc->tdesc == NULL) { - struct thread_info *saved_thread; + if (proc->attached) + { + struct thread_info *saved_thread; - saved_thread = current_thread; - current_thread = thread; + /* This needs to happen after we have attached to the + inferior and it is stopped for the first time, but + before we access any inferior registers. */ + saved_thread = current_thread; + current_thread = thread; - the_low_target.arch_setup (); + the_low_target.arch_setup (); - current_thread = saved_thread; + current_thread = saved_thread; - proc->priv->new_inferior = 0; + proc->priv->new_inferior = 0; + } + else + { + /* The process is started, but GDBserver will do + architecture-specific setup after the program stops at + the first instruction. */ + child->status_pending_p = 1; + child->status_pending = wstat; + return child; + } } } @@ -3651,6 +3671,14 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, struct thread_info *thread = get_lwp_thread (lwp); struct thread_info *saved_thread; int fast_tp_collecting; + struct process_info *proc = get_thread_process (thread); + + /* Note that target description may not be initialised + (proc->tdesc == NULL) at this point because the program hasn't + stopped at the first instruction yet. It means GDBserver skips + the extra traps from the wrapper program (see option --wrapper). + Code in this function that requires register access should be + guarded by proc->tdesc == NULL or something else. */ if (lwp->stopped == 0) return; @@ -3661,7 +3689,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, /* Cancel actions that rely on GDB not changing the PC (e.g., the user used the "jump" command, or "set $pc = foo"). */ - if (lwp->stop_pc != get_pc (lwp)) + if (thread->while_stepping != NULL && lwp->stop_pc != get_pc (lwp)) { /* Collecting 'while-stepping' actions doesn't make sense anymore. */ @@ -3787,7 +3815,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, step = 1; } - if (the_low_target.get_pc != NULL) + if (proc->tdesc != NULL && the_low_target.get_pc != NULL) { struct regcache *regcache = get_thread_regcache (current_thread, 1); @@ -4014,6 +4042,12 @@ need_step_over_p (struct inferior_list_entry *entry, void *dummy) struct lwp_info *lwp = get_thread_lwp (thread); struct thread_info *saved_thread; CORE_ADDR pc; + struct process_info *proc = get_thread_process (thread); + + /* GDBserver is skipping the extra traps from the wrapper program, + don't have to do step over. */ + if (proc->tdesc == NULL) + return 0; /* LWPs which will not be resumed are not interesting, because we might not wait for them next time through linux_wait. */ @@ -6635,6 +6669,7 @@ current_lwp_ptid (void) static struct target_ops linux_target_ops = { linux_create_inferior, + linux_arch_setup, linux_attach, linux_kill, linux_detach, diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c index ee7b28a..5cf03be 100644 --- a/gdb/gdbserver/lynx-low.c +++ b/gdb/gdbserver/lynx-low.c @@ -722,6 +722,7 @@ lynx_request_interrupt (void) static struct target_ops lynx_target_ops = { lynx_create_inferior, + NULL, /* arch_setup */ lynx_attach, lynx_kill, lynx_detach, diff --git a/gdb/gdbserver/nto-low.c b/gdb/gdbserver/nto-low.c index 9276736..19f492f 100644 --- a/gdb/gdbserver/nto-low.c +++ b/gdb/gdbserver/nto-low.c @@ -925,6 +925,7 @@ nto_supports_non_stop (void) static struct target_ops nto_target_ops = { nto_create_inferior, + NULL, /* arch_setup */ nto_attach, nto_kill, nto_detach, diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index df514f6..6b8617c 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -272,6 +272,7 @@ start_inferior (char **argv) } while (last_status.value.sig != GDB_SIGNAL_TRAP); } + target_arch_setup (); return signal_pid; } @@ -279,6 +280,8 @@ start_inferior (char **argv) (assuming success). */ last_ptid = mywait (pid_to_ptid (signal_pid), &last_status, 0, 0); + target_arch_setup (); + if (last_status.kind != TARGET_WAITKIND_EXITED && last_status.kind != TARGET_WAITKIND_SIGNALLED) { diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c index a56a889..cbee960 100644 --- a/gdb/gdbserver/spu-low.c +++ b/gdb/gdbserver/spu-low.c @@ -638,6 +638,7 @@ spu_request_interrupt (void) static struct target_ops spu_target_ops = { spu_create_inferior, + NULL, /* arch_setup */ spu_attach, spu_kill, spu_detach, diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index 9a40867..fefd8d1 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -74,6 +74,9 @@ struct target_ops int (*create_inferior) (char *program, char **args); + /* Architecture-specific setup. */ + void (*arch_setup) (void); + /* Attach to a running process. PID is the process ID to attach to, specified by the user @@ -445,6 +448,13 @@ void set_target_ops (struct target_ops *); #define create_inferior(program, args) \ (*the_target->create_inferior) (program, args) +#define target_arch_setup() \ + do \ + { \ + if (the_target->arch_setup != NULL) \ + (*the_target->arch_setup) (); \ + } while (0) + #define myattach(pid) \ (*the_target->attach) (pid) diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c index 64caf24..7ccb3dd 100644 --- a/gdb/gdbserver/win32-low.c +++ b/gdb/gdbserver/win32-low.c @@ -1785,6 +1785,7 @@ win32_get_tib_address (ptid_t ptid, CORE_ADDR *addr) static struct target_ops win32_target_ops = { win32_create_inferior, + NULL, /* arch_setup */ win32_attach, win32_kill, win32_detach,