Date: Mon, 25 Nov 2013 19:35:27 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Andriy Gapon <avg@FreeBSD.org> Cc: FreeBSD Current <freebsd-current@FreeBSD.org>, Luca Pizzamiglio <luca.pizzamiglio@gmail.com> Subject: Re: gdb has outdated knowledge of signal trampolines Message-ID: <20131125173527.GP59496@kib.kiev.ua> In-Reply-To: <529322E1.1060105@FreeBSD.org> References: <529322E1.1060105@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Aw0In/cjXooETgsz
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Mon, Nov 25, 2013 at 12:13:53PM +0200, Andriy Gapon wrote:
>=20
> It seems that placement of signal trampolines was changed a while ago. P=
ossibly
> with the introduction of the shared page, but I am not sure.
> Unfortunately, neither the gdb in base nor the ports gdb were updated to =
account
> for the new location.
>=20
> And thus, for example:
> (kgdb) bt
> #0 thr_kill () at thr_kill.S:3
> #1 0x00000008032c89a7 in nsProfileLock::FatalSignalHandler (signo=3D6,
> info=3D<optimized out>, context=3D0x7ffffb197630)
> at
> /usr/obj/ports/usr/ports/www/firefox/work/mozilla-release/obj-x86_64-unkn=
own-freebsd11.0/toolkit/profile/nsProfileLock.cpp:180
> #2 0x0000000800f90596 in handle_signal (actp=3D<optimized out>, sig=3D6,
> info=3D0x7ffffb1979a0, ucp=3D0x7ffffb197630) at /usr/src/lib/libthr/threa=
d/thr_sig.c:237
> #3 0x0000000800f9013f in thr_sighandler (sig=3D6, info=3D0x0, _ucp=3D0x7=
ffffb197630)
> at /usr/src/lib/libthr/thread/thr_sig.c:182
> #4 0x00007ffffffff003 in ?? ()
> #5 0x0000000800f90010 in ?? () at /usr/src/lib/libthr/thread/thr_sig.c:5=
66 from
> /lib/libthr.so.3
> #6 0x0000000000000000 in ?? ()
>=20
> Obviously, the gdb is confused after the frame that has 0x00007ffffffff00=
3.
>=20
> I looked only at amd64 code, but I believe that other platforms (all of t=
hem?)
> are affected as well.
>=20
> The following proof of concept patch for the base gdb seems to fix the ca=
se of
> native debugging on amd64 (target case was not tested).
>=20
> diff --git a/contrib/gdb/gdb/amd64fbsd-nat.c b/contrib/gdb/gdb/amd64fbsd-=
nat.c
> index f083734..d49dc45 100644
> --- a/contrib/gdb/gdb/amd64fbsd-nat.c
> +++ b/contrib/gdb/gdb/amd64fbsd-nat.c
> @@ -212,24 +212,23 @@ Please report this to <bug-gdb@gnu.org>.",
>=20
> SC_RBP_OFFSET =3D offset;
>=20
> - /* FreeBSD provides a kern.ps_strings sysctl that we can use to
> + /* FreeBSD provides a kern.usrstack sysctl that we can use to
> locate the sigtramp. That way we can still recognize a sigtramp
> if its location is changed in a new kernel. Of course this is
> still based on the assumption that the sigtramp is placed
> - directly under the location where the program arguments and
> - environment can be found. */
> + directly at usrstack. */
> {
> int mib[2];
> - long ps_strings;
> + long usrstack;
> size_t len;
>=20
> mib[0] =3D CTL_KERN;
> - mib[1] =3D KERN_PS_STRINGS;
> - len =3D sizeof (ps_strings);
> - if (sysctl (mib, 2, &ps_strings, &len, NULL, 0) =3D=3D 0)
> + mib[1] =3D KERN_USRSTACK;
> + len =3D sizeof (usrstack);
> + if (sysctl (mib, 2, &usrstack, &len, NULL, 0) =3D=3D 0)
> {
> - amd64fbsd_sigtramp_start_addr =3D ps_strings - 32;
> - amd64fbsd_sigtramp_end_addr =3D ps_strings;
> + amd64fbsd_sigtramp_start_addr =3D usrstack;
> + amd64fbsd_sigtramp_end_addr =3D usrstack + 0x20;
> }
> }
> }
> diff --git a/contrib/gdb/gdb/amd64fbsd-tdep.c b/contrib/gdb/gdb/amd64fbsd=
-tdep.c
> index e4e02ab..87c1484 100644
> --- a/contrib/gdb/gdb/amd64fbsd-tdep.c
> +++ b/contrib/gdb/gdb/amd64fbsd-tdep.c
> @@ -86,8 +86,8 @@ static int amd64fbsd_r_reg_offset[] =3D
> };
>=20
> /* Location of the signal trampoline. */
> -CORE_ADDR amd64fbsd_sigtramp_start_addr =3D 0x7fffffffffc0;
> -CORE_ADDR amd64fbsd_sigtramp_end_addr =3D 0x7fffffffffe0;
> +CORE_ADDR amd64fbsd_sigtramp_start_addr =3D 0x7ffffffff000;
> +CORE_ADDR amd64fbsd_sigtramp_end_addr =3D 0x7ffffffff020;
>=20
> /* From <machine/signal.h>. */
> int amd64fbsd_sc_reg_offset[] =3D
>=20
Yes, your observation is correct, but the patch could be improved.
Specifically, the location of the signal trampoline code which you
hard-coded into the patch is not guaranteed to be stable, and in fact
somewhat depends on the order of ABI sysvecs registration. The size
of the signal trampoline is not fixed as well.
Real solution is to start provide vdso for signal trampolines, which
is probably the only real reason to have vdso at all. But this is
labor-intensive and I am not convinced that the ABI changes are worth it
right now.
Instead, I propose the following sysctl helper which should provide the
same 'hackish' way to get the trampoline location, which would work
both for 64bit and 32bit, for later on i386 and amd64. For core files,
this is as bad as before since we cannot guarantee stability of the
trampoline location.
Could you update your gdb patch to use the KERN_PROC_SIGTRAMP from
the patch below ? If this works out, I will add initialization of
sv_szsigcode for ABIs which do not use shared page.
Thank you for looking into this.
sys/compat/freebsd32/freebsd32.h | 6 +++++
sys/kern/kern_proc.c | 58 ++++++++++++++++++++++++++++++++++++=
++++
sys/sys/sysctl.h | 1 +
sys/sys/user.h | 6 +++++
4 files changed, 71 insertions(+)
diff --git a/sys/compat/freebsd32/freebsd32.h b/sys/compat/freebsd32/freebs=
d32.h
index 8376e95..94f886e 100644
--- a/sys/compat/freebsd32/freebsd32.h
+++ b/sys/compat/freebsd32/freebsd32.h
@@ -362,6 +362,12 @@ struct kinfo_proc32 {
int ki_tdflags;
};
=20
+struct kinfo_sigtramp32 {
+ uint32_t ksigtramp_start;
+ uint32_t ksigtramp_end;
+ uint32_t ksigtramp_spare[4];
+};
+
struct kld32_file_stat_1 {
int version; /* set to sizeof(struct kld_file_stat_1) */
char name[MAXPATHLEN];
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 9968e76..2e6bc32 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -2632,6 +2632,60 @@ errout:
return (error);
}
=20
+static int
+sysctl_kern_proc_sigtramp(SYSCTL_HANDLER_ARGS)
+{
+ int *name =3D (int *)arg1;
+ u_int namelen =3D arg2;
+ struct proc *p;
+ struct kinfo_sigtramp kst;
+ const struct sysentvec *sv;
+ int error;
+#ifdef COMPAT_FREEBSD32
+ struct kinfo_sigtramp32 kst32;
+#endif
+
+ if (namelen !=3D 1)
+ return (EINVAL);
+
+ error =3D pget((pid_t)name[0], PGET_CANDEBUG, &p);
+ if (error !=3D 0)
+ return (error);
+ sv =3D p->p_sysent;
+#ifdef COMPAT_FREEBSD32
+ if ((req->flags & SCTL_MASK32) !=3D 0) {
+ bzero(&kst32, sizeof(kst32));
+ if (SV_PROC_FLAG(p, SV_ILP32)) {
+ if (sv->sv_sigcode_base !=3D 0) {
+ kst32.ksigtramp_start =3D sv->sv_sigcode_base;
+ kst32.ksigtramp_end =3D sv->sv_sigcode_base +
+ *sv->sv_szsigcode;
+ } else {
+ kst32.ksigtramp_start =3D sv->sv_psstrings -
+ *sv->sv_szsigcode;
+ kst32.ksigtramp_end =3D sv->sv_psstrings;
+ }
+ }
+ PROC_UNLOCK(p);
+ error =3D SYSCTL_OUT(req, &kst32, sizeof(kst32));
+ return (error);
+ }
+#endif
+ bzero(&kst, sizeof(kst));
+ if (sv->sv_sigcode_base !=3D 0) {
+ kst.ksigtramp_start =3D (char *)sv->sv_sigcode_base;
+ kst.ksigtramp_end =3D (char *)sv->sv_sigcode_base +
+ *sv->sv_szsigcode;
+ } else {
+ kst.ksigtramp_start =3D (char *)sv->sv_psstrings -
+ *sv->sv_szsigcode;
+ kst.ksigtramp_end =3D (char *)sv->sv_psstrings;
+ }
+ PROC_UNLOCK(p);
+ error =3D SYSCTL_OUT(req, &kst, sizeof(kst));
+ return (error);
+}
+
SYSCTL_NODE(_kern, KERN_PROC, proc, CTLFLAG_RD, 0, "Process table");
=20
SYSCTL_PROC(_kern_proc, KERN_PROC_ALL, all, CTLFLAG_RD|CTLTYPE_STRUCT|
@@ -2740,3 +2794,7 @@ static SYSCTL_NODE(_kern_proc, KERN_PROC_UMASK, umask=
, CTLFLAG_RD |
static SYSCTL_NODE(_kern_proc, KERN_PROC_OSREL, osrel, CTLFLAG_RW |
CTLFLAG_ANYBODY | CTLFLAG_MPSAFE, sysctl_kern_proc_osrel,
"Process binary osreldate");
+
+static SYSCTL_NODE(_kern_proc, KERN_PROC_SIGTRAMP, sigtramp, CTLFLAG_RD |
+ CTLFLAG_MPSAFE, sysctl_kern_proc_sigtramp,
+ "Process signal trampoline location");
diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h
index 64292ba..8e70a12 100644
--- a/sys/sys/sysctl.h
+++ b/sys/sys/sysctl.h
@@ -530,6 +530,7 @@ SYSCTL_ALLOWED_TYPES(UINT64, uint64_t *a; unsigned long=
long *b; );
#define KERN_PROC_PS_STRINGS 38 /* get ps_strings location */
#define KERN_PROC_UMASK 39 /* process umask */
#define KERN_PROC_OSREL 40 /* osreldate for process binary */
+#define KERN_PROC_SIGTRAMP 41 /* signal trampoline location */
=20
/*
* KERN_IPC identifiers
diff --git a/sys/sys/user.h b/sys/sys/user.h
index d2e2b6e..e926fe8 100644
--- a/sys/sys/user.h
+++ b/sys/sys/user.h
@@ -498,6 +498,12 @@ struct kinfo_kstack {
int _kkst_ispare[16]; /* Space for more stuff. */
};
=20
+struct kinfo_sigtramp {
+ void *ksigtramp_start;
+ void *ksigtramp_end;
+ void *ksigtramp_spare[4];
+};
+
#ifdef _KERNEL
/* Flags for kern_proc_out function. */
#define KERN_PROC_NOTHREADS 0x1
--Aw0In/cjXooETgsz
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)
iQIcBAEBAgAGBQJSk4peAAoJEJDCuSvBvK1B3nwQAJ5hKH7epNixI4H3qAt/fpel
X2JfUui1MbR6HpuIdTArylwwPdic2JvRhRbhdxNvEy3mbS3zSVKK32MakZvwExnt
kVh+HljDkapHg61/Z8cNM9hF94a1db45zueVm6KTXHEHx2O4fawusWQ2ShRjIsp0
ZZv2HlEa2zo+DpHUstIDcPZwjSaew7ac5ExORg32XseqrbbXWAtUUz6kzdCtEvIs
U6lK2gnFBgjdKc5hAxwOMFOlLncXT/FDQykB8W9SvqQDa147s1iNAHwTDmRlGMPa
EVWd1mjlWV7haYWomEAXMkGbSLGLDElGAMqmXJbEfWPavJXLGATBgOCgec1q1n3f
9zO2CX2xFVL48Z0txk5uhdKChntQvvU0rrSuD+6E4dLQQbAKQxTZczV4Nem8qx+j
7ED3ysDYzqnGP//U1wY/t3omfzPScOAtjZI9j5S6iSIw1Xg5a/GEO2f23uMM+8VI
RrIPES0V79J/NDjcZfR4SolFD95ocqqq3FjFH1lSLWcPZdcRtdQ6yTgSDTbPEXLM
vRP12d+JZUcesf/4BsTutP/bP3+/hiRV6NFxzFWFTL/rlghiveisgMqo/Of61ldi
G4elxwO6cU0BH0E7bnHUB+n+UZ2rxDfpWmj5IgRXTuCIRP3txjVqvg1D+D5vd5HF
paO4OEYDNZ+kECW/lpXA
=q/dW
-----END PGP SIGNATURE-----
--Aw0In/cjXooETgsz--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131125173527.GP59496>