From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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 13:08:00 -0000 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 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00697.txt.bz2 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. --=20 Yao (=E9=BD=90=E5=B0=A7) From: Yao Qi Date: Thu, 16 Jul 2015 16:35:46 +0100 Subject: [PATCH] Initialise target descrption after skipping extra traps fo= r --wrapper Nowadays, when --wrapper is used, GDBserver skips extra traps/stops in the wrapper program, and stops at the first instruction of the program to be debugged. However, GDBserver created target description in the first stop of inferior, and the executable of the inferior is the wrapper program rather than the program to be debugged. In this way, the target description can be wrong if the architectures of wrapper program and program to be debugged are different. This is shown by some fails in gdb.server/wrapper.exp on buildbot. We are testing i686-linux GDB (Fedora-i686) on an x86_64-linux box (fedora-x86-64-4) in buildbot, such configuration causes fails in gdb.server/wrapper.exp like this: spawn /home/gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuite/= ../../gdb/gdbserver/gdbserver --once --wrapper env TEST=3D1 -- :2346 /home/= gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuite/outputs/gdb.= server/wrapper/wrapper Process /home/gdb-buildbot-2/fedora-x86-64-4/fedora-i686/build/gdb/testsuit= e/outputs/gdb.server/wrapper/wrapper created; pid =3D 8795 Can't debug 64-bit process with 32-bit GDBserver Exiting target remote localhost:2346 localhost:2346: Connection timed out. (gdb) FAIL: gdb.server/wrapper.exp: setting breakpoint at marker See https://sourceware.org/ml/gdb-testers/2015-q3/msg01541.html In this case, program to be debugged ("wrapper") is 32-bit but wrapper program ("/usr/bin/env") is 64-bit, so GDBserver gets the 64-bit target description instead of 32-bit. The root cause of this problem is that GDBserver creates target description too early, and the rationale of fix could be creating target description once the GDBserver skips extra traps and inferior stops at the first instruction of the program we want to debug. IOW, when GDBserver skips extra traps, the inferior's tdesc is NULL, and mywait and its callees shouldn't use inferior's tdesc, so in this patch, we skip code that requires register access, see changes in linux_resume_one_lwp_throw and need_step_over_p. In linux_low_filter_event, if target description isn't initialised and GDBserver attached the process, we create target description immediately, because GDBserver don't have to skip extra traps for attach, IOW, it makes no sense to use --attach and --wrapper together. Otherwise, the process is launched by GDBserver, we keep the status pending, and return. After GDBserver skipped extra traps in start_inferior, we call a target_ops hook arch_setup to initialise target description there. gdb/gdbserver: 2015-07-24 Yao Qi * linux-low.c (linux_arch_setup): New function. (linux_low_filter_event): If proc->tdesc is NULL and proc->attached is true, call the_low_target.arch_setup. Otherwise, keep status pending, and return. (linux_resume_one_lwp_throw): Don't call get_pc if thread->while_stepping isn't NULL. Don't call get_thread_regcache if proc->tdesc is NULL. (need_step_over_p): Return 0 if proc->tdesc is NULL. (linux_target_ops): Install arch_setup. * server.c (start_inferior): Call the_target->arch_setup. * target.h (struct target_ops) : New field. (target_arch_setup): New marco. * lynx-low.c (lynx_target_ops): Update. * nto-low.c (nto_target_ops): Update. * spu-low.c (spu_target_ops): Update. * win32-low.c (win32_target_ops): Update. 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; } =20 +/* 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. */ =20 @@ -2105,23 +2113,35 @@ linux_low_filter_event (int lwpid, int wstat) { struct process_info *proc; =20 - /* 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 =3D find_process_pid (pid_of (thread)); - if (proc->priv->new_inferior) + if (proc->tdesc =3D=3D NULL) { - struct thread_info *saved_thread; + if (proc->attached) + { + struct thread_info *saved_thread; =20 - saved_thread =3D current_thread; - current_thread =3D 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 =3D current_thread; + current_thread =3D thread; =20 - the_low_target.arch_setup (); + the_low_target.arch_setup (); =20 - current_thread =3D saved_thread; + current_thread =3D saved_thread; =20 - proc->priv->new_inferior =3D 0; + proc->priv->new_inferior =3D 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 =3D 1; + child->status_pending =3D wstat; + return child; + } } } =20 @@ -3651,6 +3671,14 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, struct thread_info *thread =3D get_lwp_thread (lwp); struct thread_info *saved_thread; int fast_tp_collecting; + struct process_info *proc =3D get_thread_process (thread); + + /* Note that target description may not be initialised + (proc->tdesc =3D=3D 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 =3D=3D NULL or something else. */ =20 if (lwp->stopped =3D=3D 0) return; @@ -3661,7 +3689,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, =20 /* Cancel actions that rely on GDB not changing the PC (e.g., the user used the "jump" command, or "set $pc =3D foo"). */ - if (lwp->stop_pc !=3D get_pc (lwp)) + if (thread->while_stepping !=3D NULL && lwp->stop_pc !=3D 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 =3D 1; } =20 - if (the_low_target.get_pc !=3D NULL) + if (proc->tdesc !=3D NULL && the_low_target.get_pc !=3D NULL) { struct regcache *regcache =3D get_thread_regcache (current_thread, 1= ); =20 @@ -4014,6 +4042,12 @@ need_step_over_p (struct inferior_list_entry *entry,= void *dummy) struct lwp_info *lwp =3D get_thread_lwp (thread); struct thread_info *saved_thread; CORE_ADDR pc; + struct process_info *proc =3D get_thread_process (thread); + + /* GDBserver is skipping the extra traps from the wrapper program, + don't have to do step over. */ + if (proc->tdesc =3D=3D NULL) + return 0; =20 /* 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) =20 static struct target_ops linux_target_ops =3D { 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) =20 static struct target_ops lynx_target_ops =3D { 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) =20 static struct target_ops nto_target_ops =3D { 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 !=3D GDB_SIGNAL_TRAP); } + target_arch_setup (); return signal_pid; } =20 @@ -279,6 +280,8 @@ start_inferior (char **argv) (assuming success). */ last_ptid =3D mywait (pid_to_ptid (signal_pid), &last_status, 0, 0); =20 + target_arch_setup (); + if (last_status.kind !=3D TARGET_WAITKIND_EXITED && last_status.kind !=3D 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) =20 static struct target_ops spu_target_ops =3D { 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 =20 int (*create_inferior) (char *program, char **args); =20 + /* Architecture-specific setup. */ + void (*arch_setup) (void); + /* Attach to a running process. =20 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) =20 +#define target_arch_setup() \ + do \ + { \ + if (the_target->arch_setup !=3D NULL) \ + (*the_target->arch_setup) (); \ + } while (0) + #define myattach(pid) \ (*the_target->attach) (pid) =20 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) =20 static struct target_ops win32_target_ops =3D { win32_create_inferior, + NULL, /* arch_setup */ win32_attach, win32_kill, win32_detach,