Re: Base GDB tracing 32bit applications on amd64 kernel (with 64bit debugger) part 1)

[maya%netbsd.org@localhost]

Hi,

with the attached patch against -current, backtraces work again.
List of commits touched:

4b66e9c074e39cd67a49386318770ebc2a6edf82
030b2742098c17c9bd99e797fff5f1c820d6edfe
4cab1e757c7197a01b6dc268c0104fa43ab7f0aa
5542798943417f4d1bdb61d0a49a9f8c1c37445c
ef20f297e5f1527c1b54d4b2de8280a6cca21c71
80d084c7ddc1104b2bf191ccaa8a5347e71253ef


diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c
index bc2c087b7b..b0977b30b1 100644
--- a/sys/arch/amd64/amd64/machdep.c
+++ b/sys/arch/amd64/amd64/machdep.c
@@ -2095,40 +2095,49 @@ cpu_setmcontext(struct lwp *l, const mcontext_t *mcp, unsigned int flags)
 int
 cpu_mcontext_validate(struct lwp *l, const mcontext_t *mcp)
 {
-	struct proc *p __diagused = l->l_proc;
+	struct proc *p = l->l_proc;
 	struct trapframe *tf = l->l_md.md_regs;
 	const __greg_t *gr;
 	uint16_t sel;
+	const bool pk32 = (p->p_flag & PK_32) != 0;
 
-	KASSERT((p->p_flag & PK_32) == 0);
 	gr = mcp->__gregs;
 
 	if (((gr[_REG_RFLAGS] ^ tf->tf_rflags) & PSL_USERSTATIC) != 0)
 		return EINVAL;
 
+
+#define VUD(sel) (pk32 ? VALID_USER_DSEL32(sel) : VALID_USER_DSEL(sel))
+#define VUF(sel) (pk32 ? (VALID_USER_DSEL32(sel) || VALID_USER_FSEL32(sel)) \
+    : VALID_USER_DSEL(sel))
+#define VUG(sel) (pk32 ? (VALID_USER_DSEL32(sel) || VALID_USER_GSEL32(sel)) \
+    : VALID_USER_DSEL(sel))
+#define VUC(sel) (pk32 ? VALID_USER_CSEL32(sel) : VALID_USER_CSEL(sel))
+
+
 	sel = gr[_REG_ES] & 0xffff;
-	if (sel != 0 && !VALID_USER_DSEL(sel))
+	if (sel != 0 && !VUD(sel))
 		return EINVAL;
 
 	sel = gr[_REG_FS] & 0xffff;
-	if (sel != 0 && !VALID_USER_DSEL(sel))
+	if (sel != 0 && !VUF(sel))
 		return EINVAL;
 
 	sel = gr[_REG_GS] & 0xffff;
-	if (sel != 0 && !VALID_USER_DSEL(sel))
+	if (sel != 0 && !VUG(sel))
 		return EINVAL;
 
 	sel = gr[_REG_DS] & 0xffff;
-	if (!VALID_USER_DSEL(sel))
+	if (!VUD(sel))
 		return EINVAL;
 
 #ifndef XENPV
 	sel = gr[_REG_SS] & 0xffff;
-	if (!VALID_USER_DSEL(sel))
+	if (!VUD(sel))
 		return EINVAL;
 
 	sel = gr[_REG_CS] & 0xffff;
-	if (!VALID_USER_CSEL(sel))
+	if (!VUC(sel))
 		return EINVAL;
 #endif
 
diff --git a/sys/arch/amd64/amd64/process_machdep.c b/sys/arch/amd64/amd64/process_machdep.c
index c204556c91..500a96331e 100644
--- a/sys/arch/amd64/amd64/process_machdep.c
+++ b/sys/arch/amd64/amd64/process_machdep.c
@@ -100,42 +100,48 @@ process_frame(struct lwp *l)
 }
 
 int
-process_read_regs(struct lwp *l, struct reg *regs)
+process_read_regs(struct lwp *l, struct reg *regp)
 {
 	struct trapframe *tf = process_frame(l);
-	struct proc *p = l->l_proc;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
-
-	regs->regs[_REG_RDI] = tf->tf_rdi;
-	regs->regs[_REG_RSI] = tf->tf_rsi;
-	regs->regs[_REG_RDX] = tf->tf_rdx;
-	regs->regs[_REG_R10] = tf->tf_r10;
-	regs->regs[_REG_R8]  = tf->tf_r8;
-	regs->regs[_REG_R9]  = tf->tf_r9;
+	long *regs = regp->regs;
+	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
+
+	regs[_REG_RDI] = tf->tf_rdi;
+	regs[_REG_RSI] = tf->tf_rsi;
+	regs[_REG_RDX] = tf->tf_rdx;
+	regs[_REG_R10] = tf->tf_r10;
+	regs[_REG_R8]  = tf->tf_r8;
+	regs[_REG_R9]  = tf->tf_r9;
 	/* argX not touched */
-	regs->regs[_REG_RCX] = tf->tf_rcx;
-	regs->regs[_REG_R11] = tf->tf_r11;
-	regs->regs[_REG_R12] = tf->tf_r12;
-	regs->regs[_REG_R13] = tf->tf_r13;
-	regs->regs[_REG_R14] = tf->tf_r14;
-	regs->regs[_REG_R15] = tf->tf_r15;
-	regs->regs[_REG_RBP] = tf->tf_rbp;
-	regs->regs[_REG_RBX] = tf->tf_rbx;
-	regs->regs[_REG_RAX] = tf->tf_rax;
-	regs->regs[_REG_GS]  = 0;
-	regs->regs[_REG_FS]  = 0;
-	regs->regs[_REG_ES]  = GSEL(GUDATA_SEL, SEL_UPL);
-	regs->regs[_REG_DS]  = GSEL(GUDATA_SEL, SEL_UPL);
-	regs->regs[_REG_TRAPNO] = tf->tf_trapno;
-	regs->regs[_REG_ERR] = tf->tf_err;
-	regs->regs[_REG_RIP] = tf->tf_rip;
-	regs->regs[_REG_CS]  = LSEL(LUCODE_SEL, SEL_UPL);
-	regs->regs[_REG_RFLAGS] = tf->tf_rflags;
-	regs->regs[_REG_RSP] = tf->tf_rsp;
-	regs->regs[_REG_SS]  = LSEL(LUDATA_SEL, SEL_UPL);
+	regs[_REG_RCX] = tf->tf_rcx;
+	regs[_REG_R11] = tf->tf_r11;
+	regs[_REG_R12] = tf->tf_r12;
+	regs[_REG_R13] = tf->tf_r13;
+	regs[_REG_R14] = tf->tf_r14;
+	regs[_REG_R15] = tf->tf_r15;
+	regs[_REG_RBP] = tf->tf_rbp;
+	regs[_REG_RBX] = tf->tf_rbx;
+	regs[_REG_RAX] = tf->tf_rax;
+	if (pk32) {
+		regs[_REG_GS] = tf->tf_gs & 0xffff;
+		regs[_REG_FS] = tf->tf_fs & 0xffff;
+		regs[_REG_ES] = tf->tf_es & 0xffff;
+		regs[_REG_DS] = tf->tf_ds & 0xffff;
+		regs[_REG_CS] = tf->tf_cs & 0xffff;
+		regs[_REG_SS] = tf->tf_ss & 0xffff;
+	} else {
+		regs[_REG_GS] = 0;
+		regs[_REG_FS] = 0;
+		regs[_REG_ES] = GSEL(GUDATA_SEL, SEL_UPL);
+		regs[_REG_DS] = GSEL(GUDATA_SEL, SEL_UPL);
+		regs[_REG_CS] = LSEL(LUCODE_SEL, SEL_UPL);
+		regs[_REG_SS] = LSEL(LUDATA_SEL, SEL_UPL);
+	}
+	regs[_REG_TRAPNO] = tf->tf_trapno;
+	regs[_REG_ERR] = tf->tf_err;
+	regs[_REG_RIP] = tf->tf_rip;
+	regs[_REG_RFLAGS] = tf->tf_rflags;
+	regs[_REG_RSP] = tf->tf_rsp;
 
 	return 0;
 }
@@ -143,11 +149,6 @@ process_read_regs(struct lwp *l, struct reg *regs)
 int
 process_read_fpregs(struct lwp *l, struct fpreg *regs, size_t *sz)
 {
-	struct proc *p = l->l_proc;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
 
 	process_read_fpregs_xmm(l, &regs->fxstate);
 
@@ -157,11 +158,6 @@ process_read_fpregs(struct lwp *l, struct fpreg *regs, size_t *sz)
 int
 process_read_dbregs(struct lwp *l, struct dbreg *regs, size_t *sz)
 {
-	struct proc *p = l->l_proc;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
 
 	x86_dbregs_read(l, regs);
 
@@ -172,13 +168,9 @@ int
 process_write_regs(struct lwp *l, const struct reg *regp)
 {
 	struct trapframe *tf = process_frame(l);
-	struct proc *p = l->l_proc;
 	int error;
 	const long *regs = regp->regs;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
+	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
 
 	/*
 	 * Check for security violations. Note that struct regs is compatible
@@ -204,16 +196,25 @@ process_write_regs(struct lwp *l, const struct reg *regp)
 	tf->tf_rbp  = regs[_REG_RBP];
 	tf->tf_rbx  = regs[_REG_RBX];
 	tf->tf_rax  = regs[_REG_RAX];
-	tf->tf_gs   = 0;
-	tf->tf_fs   = 0;
-	tf->tf_es   = GSEL(GUDATA_SEL, SEL_UPL);
-	tf->tf_ds   = GSEL(GUDATA_SEL, SEL_UPL);
+	if (pk32) {
+		tf->tf_gs = regs[_REG_GS] & 0xffff;
+		tf->tf_fs = regs[_REG_FS] & 0xffff;
+		tf->tf_es = regs[_REG_ES] & 0xffff;
+		tf->tf_ds = regs[_REG_DS] & 0xffff;
+		tf->tf_cs = regs[_REG_CS] & 0xffff;
+		tf->tf_ss = regs[_REG_SS] & 0xffff;
+	} else {
+		tf->tf_gs = 0;
+		tf->tf_fs = 0;
+		tf->tf_es = GSEL(GUDATA_SEL, SEL_UPL);
+		tf->tf_ds = GSEL(GUDATA_SEL, SEL_UPL);
+		tf->tf_cs = LSEL(LUCODE_SEL, SEL_UPL);
+		tf->tf_ss = LSEL(LUDATA_SEL, SEL_UPL);
+	}
 	/* trapno, err not touched */
 	tf->tf_rip  = regs[_REG_RIP];
-	tf->tf_cs   = LSEL(LUCODE_SEL, SEL_UPL);
 	tf->tf_rflags = regs[_REG_RFLAGS];
 	tf->tf_rsp  = regs[_REG_RSP];
-	tf->tf_ss   = LSEL(LUDATA_SEL, SEL_UPL);
 
 #ifdef XENPV
 	/* see comment in cpu_setmcontext */
@@ -227,11 +228,6 @@ process_write_regs(struct lwp *l, const struct reg *regp)
 int
 process_write_fpregs(struct lwp *l, const struct fpreg *regs, size_t sz)
 {
-	struct proc *p = l->l_proc;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
 
 	process_write_fpregs_xmm(l, &regs->fxstate);
 	return 0;
@@ -240,13 +236,8 @@ process_write_fpregs(struct lwp *l, const struct fpreg *regs, size_t sz)
 int
 process_write_dbregs(struct lwp *l, const struct dbreg *regs, size_t sz)
 {
-	struct proc *p = l->l_proc;
 	int error;
 
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
-
 	/*
 	 * Check for security violations.
 	 */
@@ -276,11 +267,6 @@ int
 process_set_pc(struct lwp *l, void *addr)
 {
 	struct trapframe *tf = process_frame(l);
-	struct proc *p = l->l_proc;
-
-	if (p->p_flag & PK_32) {
-		return EINVAL;
-	}
 
 	if ((uint64_t)addr >= VM_MAXUSER_ADDRESS)
 		return EINVAL;
diff --git a/sys/kern/sys_ptrace_common.c b/sys/kern/sys_ptrace_common.c
index d77a3453d8..b7f3e25ff3 100644
--- a/sys/kern/sys_ptrace_common.c
+++ b/sys/kern/sys_ptrace_common.c
@@ -124,7 +124,6 @@ __KERNEL_RCSID(0, "$NetBSD: sys_ptrace_common.c,v 1.55 2019/06/11 23:18:55 kamil
 #include "opt_ptrace.h"
 #include "opt_ktrace.h"
 #include "opt_pax.h"
-#include "opt_compat_netbsd32.h"
 #endif
 
 #if defined(__HAVE_COMPAT_NETBSD32) && !defined(COMPAT_NETBSD32) \
@@ -820,7 +819,7 @@ ptrace_regs(struct lwp *l, struct lwp **lt, int rq, struct ptrace_methods *ptm,
     void *addr, size_t data)
 {
 	int error;
-	struct proc *t = (*lt)->l_proc;
+	struct proc *t = l->l_proc;
 	struct vmspace *vm;
 
 	if ((error = ptrace_update_lwp(t, lt, data)) != 0)
@@ -864,7 +863,7 @@ ptrace_regs(struct lwp *l, struct lwp **lt, int rq, struct ptrace_methods *ptm,
 		return EINVAL;
 	}
 
-	error = proc_vmspace_getref(l->l_proc, &vm);
+	error = proc_vmspace_getref(t, &vm);
 	if (error)
 		return error;
 
@@ -1432,71 +1431,40 @@ out:
 	return error;
 }
 
-typedef int (*regrfunc_t)(struct lwp *, void *, size_t *);
-typedef int (*regwfunc_t)(struct lwp *, void *, size_t);
-
-#ifdef PT_REGISTERS
-static int
-proc_regio(struct lwp *l, struct uio *uio, size_t ks, regrfunc_t r,
-    regwfunc_t w)
+int
+process_doregs(struct lwp *curl /*tracer*/,
+    struct lwp *l /*traced*/,
+    struct uio *uio)
 {
-	char buf[1024];
+#if defined(PT_GETREGS) || defined(PT_SETREGS)
 	int error;
+	struct reg r;
 	char *kv;
-	size_t kl;
-
-	if (ks > sizeof(buf))
-		return E2BIG;
+	int kl;
 
-	if (uio->uio_offset < 0 || uio->uio_offset > (off_t)ks)
+	if (uio->uio_offset < 0 || uio->uio_offset > (off_t)sizeof(r))
 		return EINVAL;
 
-	kv = buf + uio->uio_offset;
-	kl = ks - uio->uio_offset;
+	kl = sizeof(r);
+	kv = (char *)&r;
 
-	if (kl > uio->uio_resid)
+	kv += uio->uio_offset;
+	kl -= uio->uio_offset;
+	if ((size_t)kl > uio->uio_resid)
 		kl = uio->uio_resid;
 
-	error = (*r)(l, buf, &ks);
+	error = process_read_regs(l, &r);
 	if (error == 0)
 		error = uiomove(kv, kl, uio);
 	if (error == 0 && uio->uio_rw == UIO_WRITE) {
 		if (l->l_stat != LSSTOP)
 			error = EBUSY;
 		else
-			error = (*w)(l, buf, ks);
+			error = process_write_regs(l, &r);
 	}
 
 	uio->uio_offset = 0;
 	return error;
-}
-#endif
-
-int
-process_doregs(struct lwp *curl /*tracer*/,
-    struct lwp *l /*traced*/,
-    struct uio *uio)
-{
-#if defined(PT_GETREGS) || defined(PT_SETREGS)
-	size_t s;
-	regrfunc_t r;
-	regwfunc_t w;
-
-#ifdef COMPAT_NETBSD32
-	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
-
-	if (__predict_false(pk32)) {
-		s = sizeof(process_reg32);
-		r = (regrfunc_t)process_read_regs32;
-		w = (regwfunc_t)process_write_regs32;
-	} else
-#endif
-	{
-		s = sizeof(struct reg);
-		r = (regrfunc_t)process_read_regs;
-		w = (regwfunc_t)process_write_regs;
-	}
-	return proc_regio(l, uio, s, r, w);
 #else
 	return EINVAL;
 #endif
@@ -1519,25 +1487,33 @@ process_dofpregs(struct lwp *curl /*tracer*/,
     struct uio *uio)
 {
 #if defined(PT_GETFPREGS) || defined(PT_SETFPREGS)
-	size_t s;
-	regrfunc_t r;
-	regwfunc_t w;
-
-#ifdef COMPAT_NETBSD32
-	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
-
-	if (__predict_false(pk32)) {
-		s = sizeof(process_fpreg32);
-		r = (regrfunc_t)process_read_fpregs32;
-		w = (regwfunc_t)process_write_fpregs32;
-	} else
-#endif
-	{
-		s = sizeof(struct fpreg);
-		r = (regrfunc_t)process_read_fpregs;
-		w = (regwfunc_t)process_write_fpregs;
+	int error;
+	struct fpreg r;
+	char *kv;
+	size_t kl;
+
+	if (uio->uio_offset < 0 || uio->uio_offset > (off_t)sizeof(r))
+		return EINVAL;
+
+	kl = sizeof(r);
+	kv = (char *)&r;
+
+	kv += uio->uio_offset;
+	kl -= uio->uio_offset;
+	if (kl > uio->uio_resid)
+		kl = uio->uio_resid;
+
+	error = process_read_fpregs(l, &r, &kl);
+	if (error == 0)
+		error = uiomove(kv, kl, uio);
+	if (error == 0 && uio->uio_rw == UIO_WRITE) {
+		if (l->l_stat != LSSTOP)
+			error = EBUSY;
+		else
+			error = process_write_fpregs(l, &r, kl);
 	}
-	return proc_regio(l, uio, s, r, w);
+	uio->uio_offset = 0;
+	return error;
 #else
 	return EINVAL;
 #endif
@@ -1560,25 +1536,33 @@ process_dodbregs(struct lwp *curl /*tracer*/,
     struct uio *uio)
 {
 #if defined(PT_GETDBREGS) || defined(PT_SETDBREGS)
-	size_t s;
-	regrfunc_t r;
-	regwfunc_t w;
-
-#ifdef COMPAT_NETBSD32
-	const bool pk32 = (l->l_proc->p_flag & PK_32) != 0;
-
-	if (__predict_false(pk32)) {
-		s = sizeof(process_dbreg32);
-		r = (regrfunc_t)process_read_dbregs32;
-		w = (regwfunc_t)process_write_dbregs32;
-	} else
-#endif
-	{
-		s = sizeof(struct dbreg);
-		r = (regrfunc_t)process_read_dbregs;
-		w = (regwfunc_t)process_write_dbregs;
+	int error;
+	struct dbreg r;
+	char *kv;
+	size_t kl;
+
+	if (uio->uio_offset < 0 || uio->uio_offset > (off_t)sizeof(r))
+		return EINVAL;
+
+	kl = sizeof(r);
+	kv = (char *)&r;
+
+	kv += uio->uio_offset;
+	kl -= uio->uio_offset;
+	if (kl > uio->uio_resid)
+		kl = uio->uio_resid;
+
+	error = process_read_dbregs(l, &r, &kl);
+	if (error == 0)
+		error = uiomove(kv, kl, uio);
+	if (error == 0 && uio->uio_rw == UIO_WRITE) {
+		if (l->l_stat != LSSTOP)
+			error = EBUSY;
+		else
+			error = process_write_dbregs(l, &r, kl);
 	}
-	return proc_regio(l, uio, s, r, w);
+	uio->uio_offset = 0;
+	return error;
 #else
 	return EINVAL;
 #endif