On Mon, Nov 25, 2013 at 12:13:53PM +0200, Andriy Gapon wrote:
>
> It seems that placement of signal trampolines was changed a while ago.
> Possibly
> 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.
>
> And thus, for example:
> (kgdb) bt
> #0 thr_kill () at thr_kill.S:3
> #1 0x00000008032c89a7 in nsProfileLock::FatalSignalHandler (signo=6,
> info=<optimized out>, context=0x7ffffb197630)
> at
> /usr/obj/ports/usr/ports/www/firefox/work/mozilla-release/obj-x86_64-unknown-freebsd11.0/toolkit/profile/nsProfileLock.cpp:180
> #2 0x0000000800f90596 in handle_signal (actp=<optimized out>, sig=6,
> info=0x7ffffb1979a0, ucp=0x7ffffb197630) at
> /usr/src/lib/libthr/thread/thr_sig.c:237
> #3 0x0000000800f9013f in thr_sighandler (sig=6, info=0x0,
> _ucp=0x7ffffb197630)
> 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:566
> from
> /lib/libthr.so.3
> #6 0x0000000000000000 in ?? ()
>
> Obviously, the gdb is confused after the frame that has 0x00007ffffffff003.
>
> I looked only at amd64 code, but I believe that other platforms (all of them?)
> are affected as well.
>
> The following proof of concept patch for the base gdb seems to fix the case of
> native debugging on amd64 (target case was not tested).
>
> 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-...@gnu.org>.",
>
> SC_RBP_OFFSET = offset;
>
> - /* 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;
>
> mib[0] = CTL_KERN;
> - mib[1] = KERN_PS_STRINGS;
> - len = sizeof (ps_strings);
> - if (sysctl (mib, 2, &ps_strings, &len, NULL, 0) == 0)
> + mib[1] = KERN_USRSTACK;
> + len = sizeof (usrstack);
> + if (sysctl (mib, 2, &usrstack, &len, NULL, 0) == 0)
> {
> - amd64fbsd_sigtramp_start_addr = ps_strings - 32;
> - amd64fbsd_sigtramp_end_addr = ps_strings;
> + amd64fbsd_sigtramp_start_addr = usrstack;
> + amd64fbsd_sigtramp_end_addr = 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[] =
> };
>
> /* Location of the signal trampoline. */
> -CORE_ADDR amd64fbsd_sigtramp_start_addr = 0x7fffffffffc0;
> -CORE_ADDR amd64fbsd_sigtramp_end_addr = 0x7fffffffffe0;
> +CORE_ADDR amd64fbsd_sigtramp_start_addr = 0x7ffffffff000;
> +CORE_ADDR amd64fbsd_sigtramp_end_addr = 0x7ffffffff020;
>
> /* From <machine/signal.h>. */
> int amd64fbsd_sc_reg_offset[] =
>
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/freebsd32.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;
};
+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);
}
+static int
+sysctl_kern_proc_sigtramp(SYSCTL_HANDLER_ARGS)
+{
+ int *name = (int *)arg1;
+ u_int namelen = arg2;
+ struct proc *p;
+ struct kinfo_sigtramp kst;
+ const struct sysentvec *sv;
+ int error;
+#ifdef COMPAT_FREEBSD32
+ struct kinfo_sigtramp32 kst32;
+#endif
+
+ if (namelen != 1)
+ return (EINVAL);
+
+ error = pget((pid_t)name[0], PGET_CANDEBUG, &p);
+ if (error != 0)
+ return (error);
+ sv = p->p_sysent;
+#ifdef COMPAT_FREEBSD32
+ if ((req->flags & SCTL_MASK32) != 0) {
+ bzero(&kst32, sizeof(kst32));
+ if (SV_PROC_FLAG(p, SV_ILP32)) {
+ if (sv->sv_sigcode_base != 0) {
+ kst32.ksigtramp_start = sv->sv_sigcode_base;
+ kst32.ksigtramp_end = sv->sv_sigcode_base +
+ *sv->sv_szsigcode;
+ } else {
+ kst32.ksigtramp_start = sv->sv_psstrings -
+ *sv->sv_szsigcode;
+ kst32.ksigtramp_end = sv->sv_psstrings;
+ }
+ }
+ PROC_UNLOCK(p);
+ error = SYSCTL_OUT(req, &kst32, sizeof(kst32));
+ return (error);
+ }
+#endif
+ bzero(&kst, sizeof(kst));
+ if (sv->sv_sigcode_base != 0) {
+ kst.ksigtramp_start = (char *)sv->sv_sigcode_base;
+ kst.ksigtramp_end = (char *)sv->sv_sigcode_base +
+ *sv->sv_szsigcode;
+ } else {
+ kst.ksigtramp_start = (char *)sv->sv_psstrings -
+ *sv->sv_szsigcode;
+ kst.ksigtramp_end = (char *)sv->sv_psstrings;
+ }
+ PROC_UNLOCK(p);
+ error = SYSCTL_OUT(req, &kst, sizeof(kst));
+ return (error);
+}
+
SYSCTL_NODE(_kern, KERN_PROC, proc, CTLFLAG_RD, 0, "Process table");
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 */
/*
* 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. */
};
+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
pgpABUDlaWLo1.pgp
Description: PGP signature
