Port-i386 archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: [patches 2/2] struct switchframe/pcb improvements
Hi all,
I discovered a couple of issues in one of the patches I sent. I've
attached a new version of the patch, and commented on the issues below:
On 2014-01-09 17:41, Richard Hansen wrote:
[snip]
> commit 6b920b1893d33337eb86c45804686bb481a45385
> Author: Richard Hansen <rhansen%bbn.com@localhost>
> Date: Thu Oct 3 22:24:54 2013 +0000
>
> i386: remove pcb_ebp member from struct pcb
>
> Now that %ebp is stored in struct switchframe, remove it from struct
> pcb.
>
> This changes the layout of the process control block, which affects
> tools that examine LWPs in crash dump files (or /dev/kmem). Adjust
> the gdb and crash utilities to correctly parse the new memory layout.
>
[snip]
> diff --git a/src/sys/arch/x86/x86/db_trace.c b/src/sys/arch/x86/x86/db_trace.c
> index 5b8e14f..c8ba951 100644
> --- a/src/sys/arch/x86/x86/db_trace.c
> +++ b/src/sys/arch/x86/x86/db_trace.c
> @@ -169,7 +169,14 @@ db_stack_trace_print(db_expr_t addr, bool have_addr,
> db_expr_t count,
> else
> #endif
> {
> - db_read_bytes((db_addr_t)&pcb->pcb_bp,
> +#ifdef __x86_64__
> + db_addr_t frameaddr = (db_addr_t)&pcb->pcb_rbp;
> +#else
> + db_addr_t frameaddr = (db_addr_t)
> + ((char *)pcb->pcb_sp
> + + offsetof(struct switchframe, sf_ebp));
While the above three lines are technically correct, a less convoluted
way of accomplishing the same thing is:
db_addr_t frameaddr = (db_addr_t)
&((struct switchframe *)pcb->pcb_esp)->sf_ebp;
> +#endif
> + db_read_bytes(frameaddr,
> sizeof(frame), (char *)&frame);
> db_read_bytes((db_addr_t)(frame + 1),
> sizeof(callpc), (char *)&callpc);
[snip]
> diff --git a/src/usr.sbin/crash/arch/x86.c b/src/usr.sbin/crash/arch/x86.c
> index 94381fb..80582d0 100644
> --- a/src/usr.sbin/crash/arch/x86.c
> +++ b/src/usr.sbin/crash/arch/x86.c
> @@ -66,7 +66,11 @@ db_mach_init(kvm_t *kd)
> errx(EXIT_FAILURE, "cannot read dumppcb: %s", kvm_geterr(kd));
> }
> ddb_regs.tf_sp = pcb.pcb_sp;
> - ddb_regs.tf_bp = pcb.pcb_bp;
> +#ifdef __x86_64__
> + ddb_regs.tf_bp = pcb.pcb_rbp;
> +#else
> + ddb_regs.tf_bp = pcb.pcb_sp + offsetof(struct switchframe, sf_ebp);
The above line is broken -- the value must be dereferenced. Here's a
fixed version of the line:
ddb_regs.tf_bp = ((struct switchframe *)pcb.pcb_esp)->sf_ebp;
> +#endif
> if (ddb_regs.tf_bp != 0 && ddb_regs.tf_sp != 0) {
> printf("Backtrace from time of crash is available.\n");
> }
[snip]
-Richard
From 08a8ac1369a085af9b3daa183875553b4d7873d5 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen%bbn.com@localhost>
Date: Thu, 3 Oct 2013 22:24:54 +0000
Subject: [PATCH] i386: remove pcb_ebp member from struct pcb
Now that %ebp is stored in struct switchframe, remove it from struct
pcb.
This changes the layout of the process control block, which affects
tools that examine LWPs in crash dump files (or /dev/kmem). Adjust
the gdb and crash utilities to correctly parse the new memory layout.
---
src/external/gpl3/gdb/dist/gdb/i386nbsd-nat.c | 10 ++++++----
src/sys/arch/i386/i386/genassym.cf | 1 -
src/sys/arch/i386/i386/locore.S | 4 ----
src/sys/arch/i386/i386/mptramp.S | 1 -
src/sys/arch/i386/i386/spl.S | 1 -
src/sys/arch/i386/include/pcb.h | 1 -
src/sys/arch/x86/include/db_machdep.h | 2 --
src/sys/arch/x86/x86/db_trace.c | 8 +++++++-
src/sys/arch/x86/x86/vm_machdep.c | 1 -
src/sys/sys/param.h | 2 +-
src/usr.sbin/crash/arch/x86.c | 6 +++++-
11 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/src/external/gpl3/gdb/dist/gdb/i386nbsd-nat.c
b/src/external/gpl3/gdb/dist/gdb/i386nbsd-nat.c
index 0c07c4e..ddf7fa0 100644
--- a/src/external/gpl3/gdb/dist/gdb/i386nbsd-nat.c
+++ b/src/external/gpl3/gdb/dist/gdb/i386nbsd-nat.c
@@ -79,6 +79,8 @@ i386nbsd_supply_pcb (struct regcache *regcache, struct pcb
*pcb)
%ebx
%ebp
return address
+
+ NetBSD 6.99.30 removed %ebp from the pcb.
*/
/* The stack pointer shouldn't be zero. */
@@ -95,7 +97,7 @@ i386nbsd_supply_pcb (struct regcache *regcache, struct pcb
*pcb)
pcb->pcb_esp += sizeof (struct switchframe);
regcache_raw_supply (regcache, I386_EDI_REGNUM, &sf.sf_edi);
regcache_raw_supply (regcache, I386_ESI_REGNUM, &sf.sf_esi);
- regcache_raw_supply (regcache, I386_EBP_REGNUM, &pcb->pcb_ebp);
+ regcache_raw_supply (regcache, I386_EBP_REGNUM, &sf.sf_ebp);
regcache_raw_supply (regcache, I386_ESP_REGNUM, &pcb->pcb_esp);
regcache_raw_supply (regcache, I386_EBX_REGNUM, &sf.sf_ebx);
regcache_raw_supply (regcache, I386_EIP_REGNUM, &sf.sf_eip);
@@ -107,10 +109,10 @@ i386nbsd_supply_pcb (struct regcache *regcache, struct
pcb *pcb)
/* No, the pcb must have been last updated by savectx() in old
dumpsys(). Use the frame pointer to recover enough state. */
- read_memory (pcb->pcb_ebp, (gdb_byte *) &fp, sizeof(fp));
- read_memory (pcb->pcb_ebp + 4, (gdb_byte *) &pc, sizeof(pc));
+ read_memory (sf.sf_ebp, (gdb_byte *) &fp, sizeof(fp));
+ read_memory (sf.sf_ebp + 4, (gdb_byte *) &pc, sizeof(pc));
- regcache_raw_supply (regcache, I386_ESP_REGNUM, &pcb->pcb_ebp);
+ regcache_raw_supply (regcache, I386_ESP_REGNUM, &sf.sf_ebp);
regcache_raw_supply (regcache, I386_EBP_REGNUM, &fp);
regcache_raw_supply (regcache, I386_EIP_REGNUM, &pc);
}
diff --git a/src/sys/arch/i386/i386/genassym.cf
b/src/sys/arch/i386/i386/genassym.cf
index c762f61..dac7d1e 100644
--- a/src/sys/arch/i386/i386/genassym.cf
+++ b/src/sys/arch/i386/i386/genassym.cf
@@ -199,7 +199,6 @@ define IP6_SRC offsetof(struct
ip6_hdr, ip6_src)
define IP6_DST offsetof(struct ip6_hdr, ip6_dst)
define PCB_CR3 offsetof(struct pcb, pcb_cr3)
-define PCB_EBP offsetof(struct pcb, pcb_ebp)
define PCB_ESP offsetof(struct pcb, pcb_esp)
define PCB_ESP0 offsetof(struct pcb, pcb_esp0)
define PCB_CR0 offsetof(struct pcb, pcb_cr0)
diff --git a/src/sys/arch/i386/i386/locore.S b/src/sys/arch/i386/i386/locore.S
index e6e1730..31464d6 100644
--- a/src/sys/arch/i386/i386/locore.S
+++ b/src/sys/arch/i386/i386/locore.S
@@ -970,7 +970,6 @@ ENTRY(dumpsys)
pushl %esi
pushl %edi
movl %esp,_C_LABEL(dumppcb)+PCB_ESP
- movl %ebp,_C_LABEL(dumppcb)+PCB_EBP
call _C_LABEL(dodumpsys) # dump!
addl $(3*4), %esp # unwind switchframe
popl %ebp
@@ -1015,11 +1014,9 @@ ENTRY(cpu_switchto)
/* Save old context. */
movl L_PCB(%esi),%eax
movl %esp,PCB_ESP(%eax)
- movl %ebp,PCB_EBP(%eax)
/* Switch to newlwp's stack. */
1: movl L_PCB(%edi),%ebx
- movl PCB_EBP(%ebx),%ebp
movl PCB_ESP(%ebx),%esp
/*
@@ -1146,7 +1143,6 @@ END(cpu_switchto)
ENTRY(savectx)
movl 4(%esp),%edx # edx = pcb
movl %esp,PCB_ESP(%edx)
- movl %ebp,PCB_EBP(%edx)
ret
END(savectx)
diff --git a/src/sys/arch/i386/i386/mptramp.S b/src/sys/arch/i386/i386/mptramp.S
index d643648..10dcfd8 100644
--- a/src/sys/arch/i386/i386/mptramp.S
+++ b/src/sys/arch/i386/i386/mptramp.S
@@ -251,7 +251,6 @@ mp_cont:
HALTT(0x19, %esi)
movl PCB_ESP(%esi),%esp
- movl PCB_EBP(%esi),%ebp
HALT(0x20)
/* Switch address space. */
diff --git a/src/sys/arch/i386/i386/spl.S b/src/sys/arch/i386/i386/spl.S
index d56797f..002aa90 100644
--- a/src/sys/arch/i386/i386/spl.S
+++ b/src/sys/arch/i386/i386/spl.S
@@ -382,7 +382,6 @@ IDTVEC(softintr)
movl L_PCB(%edi),%edx
movl L_PCB(%esi),%ecx
movl %esp,PCB_ESP(%ecx)
- movl %ebp,PCB_EBP(%ecx)
movl PCB_ESP0(%edx),%esp /* onto new stack */
sti
pushl IS_MAXLEVEL(%eax) /* ipl to run at */
diff --git a/src/sys/arch/i386/include/pcb.h b/src/sys/arch/i386/include/pcb.h
index f3f9c47..e079116 100644
--- a/src/sys/arch/i386/include/pcb.h
+++ b/src/sys/arch/i386/include/pcb.h
@@ -84,7 +84,6 @@
struct pcb {
int pcb_esp0; /* ring0 esp */
int pcb_esp; /* kernel esp */
- int pcb_ebp; /* kernel ebp */
int pcb_unused; /* unused */
int pcb_cr0; /* saved image of CR0 */
int pcb_cr2; /* page fault address (CR2) */
diff --git a/src/sys/arch/x86/include/db_machdep.h
b/src/sys/arch/x86/include/db_machdep.h
index d302fc1..bac90c9 100644
--- a/src/sys/arch/x86/include/db_machdep.h
+++ b/src/sys/arch/x86/include/db_machdep.h
@@ -21,14 +21,12 @@ struct db_variable;
#define tf_sp tf_rsp
#define tf_ip tf_rip
#define tf_bp tf_rbp
-#define pcb_bp pcb_rbp
#define pcb_sp pcb_rsp
#define x86_frame x86_64_frame
#else
#define tf_sp tf_esp
#define tf_ip tf_eip
#define tf_bp tf_ebp
-#define pcb_bp pcb_ebp
#define pcb_sp pcb_esp
#define x86_frame i386_frame
#endif
diff --git a/src/sys/arch/x86/x86/db_trace.c b/src/sys/arch/x86/x86/db_trace.c
index 5b8e14f..7d7dc19 100644
--- a/src/sys/arch/x86/x86/db_trace.c
+++ b/src/sys/arch/x86/x86/db_trace.c
@@ -169,7 +169,13 @@ db_stack_trace_print(db_expr_t addr, bool have_addr,
db_expr_t count,
else
#endif
{
- db_read_bytes((db_addr_t)&pcb->pcb_bp,
+#ifdef __x86_64__
+ db_addr_t frameaddr = (db_addr_t)&pcb->pcb_rbp;
+#else
+ db_addr_t frameaddr = (db_addr_t)
+ &((struct switchframe *)pcb->pcb_esp)->sf_ebp;
+#endif
+ db_read_bytes(frameaddr,
sizeof(frame), (char *)&frame);
db_read_bytes((db_addr_t)(frame + 1),
sizeof(callpc), (char *)&callpc);
diff --git a/src/sys/arch/x86/x86/vm_machdep.c
b/src/sys/arch/x86/x86/vm_machdep.c
index ac2d793..13e8839 100644
--- a/src/sys/arch/x86/x86/vm_machdep.c
+++ b/src/sys/arch/x86/x86/vm_machdep.c
@@ -238,7 +238,6 @@ cpu_lwp_fork(struct lwp *l1, struct lwp *l2, void *stack,
size_t stacksize,
sf->sf_ebp = (int)l2;
sf->sf_eip = (int)lwp_trampoline;
pcb2->pcb_esp = (int)sf;
- pcb2->pcb_ebp = (int)l2;
#endif
}
diff --git a/src/sys/sys/param.h b/src/sys/sys/param.h
index 0280d44..ca65166 100644
--- a/src/sys/sys/param.h
+++ b/src/sys/sys/param.h
@@ -63,7 +63,7 @@
* 2.99.9 (299000900)
*/
-#define __NetBSD_Version__ 699002900 /* NetBSD 6.99.29 */
+#define __NetBSD_Version__ 699003000 /* NetBSD 6.99.30 */
#define __NetBSD_Prereq__(M,m,p) (((((M) * 100000000) + \
(m) * 1000000) + (p) * 100) <= __NetBSD_Version__)
diff --git a/src/usr.sbin/crash/arch/x86.c b/src/usr.sbin/crash/arch/x86.c
index 94381fb..e4d7d13 100644
--- a/src/usr.sbin/crash/arch/x86.c
+++ b/src/usr.sbin/crash/arch/x86.c
@@ -66,7 +66,11 @@ db_mach_init(kvm_t *kd)
errx(EXIT_FAILURE, "cannot read dumppcb: %s", kvm_geterr(kd));
}
ddb_regs.tf_sp = pcb.pcb_sp;
- ddb_regs.tf_bp = pcb.pcb_bp;
+#ifdef __x86_64__
+ ddb_regs.tf_bp = pcb.pcb_rbp;
+#else
+ ddb_regs.tf_bp = ((struct switchframe *)pcb.pcb_esp)->sf_ebp;
+#endif
if (ddb_regs.tf_bp != 0 && ddb_regs.tf_sp != 0) {
printf("Backtrace from time of crash is available.\n");
}
--
1.8.5.4
Home |
Main Index |
Thread Index |
Old Index