> Date: Sun, 3 Apr 2016 00:39:14 -0700
> From: Philip Guenther <guent...@gmail.com>
> 
> On Sat, 2 Apr 2016, Philip Guenther wrote:
> > I ask as I see some "this can't work" logic between the kernel and gdb for 
> > at least i386 and possibly others and I'd like to hear what *is* working 
> > that I should verify I don't break when making changes...
> 
> Here's a quick fix for the initial bug that caught my eye.  On i386, 
> savectx() sets PCB_SAVECTX in pcb_flags.  Okay, that's nice, but
> 
> a) savectx() is called from cpu_fork(), which results in the flag being
>    set on both parent and child, and
> 
> b) nothing clears it
> 
> Result is that it's set on *all* threads, which makes gdb's "kvm proc" 
> never show the frame of mi_switch(), as it thinks this pcb is always from 
> savectx().
> 
> Diff below fixes that by eliminating the savectx() call in cpu_fork().  
> Turns out that call is completely unnecessary: savectx() only updates 
> pcb_esp and pcb_ebp (and the PCB_SAVECTX flag), but cpu_fork() overwrites 
> both those values in the child's pcb, while the values in the parent are 
> invalid as soon as it returns.
> 
> before:
> (gdb) bt
> #0  0xd03bf814 in sleep_finish (sls=0x0, do_sleep=1)
>     at ../../../../kern/kern_synch.c:266
> #1  0xd03bfddf in tsleep (ident=0xd0b8ae44, priority=288,
>     wmesg=0xd09e9690 "nanosleep", timo=0) at 
> ../../../../kern/kern_synch.c:148
> #2  0xd03c1faa in sys_nanosleep (p=0xd7203cdc, v=0xf5d5cf5c, 
> retval=0xf5d5cf7c)
>     at ../../../../kern/kern_time.c:289
> 
> after:
> (gdb) bt
> #0  mi_switch () at cpu.h:208
> #1  0xd03bf814 in sleep_finish (sls=0xf5d84e7c, do_sleep=1)
>     at ../../../../kern/kern_synch.c:266
> #2  0xd03bfddf in tsleep (ident=0xd0b8ae44, priority=288,
>     wmesg=0xd09e9690 "nanosleep", timo=600001)
>     at ../../../../kern/kern_synch.c:148
> #3  0xd03c1faa in sys_nanosleep (p=0xd7204b70, v=0xf5d84f5c, 
> retval=0xf5d84f7c)
>     at ../../../../kern/kern_time.c:289
> 
> Note the bogus sls=0x0 in before case.
> 
> ok?

ok kettenis@

> Index: vm_machdep.c
> ===================================================================
> RCS file: /data/src/openbsd/src/sys/arch/i386/i386/vm_machdep.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 vm_machdep.c
> --- vm_machdep.c      5 May 2015 02:13:46 -0000       1.63
> +++ vm_machdep.c      3 Apr 2016 07:19:50 -0000
> @@ -85,13 +85,8 @@ cpu_fork(struct proc *p1, struct proc *p
>  
>       p2->p_md.md_flags = p1->p_md.md_flags;
>  
> -     /* Copy pcb from proc p1 to p2. */
> -     if (p1 == curproc) {
> -             /* Sync the PCB before we copy it. */
> -             savectx(curpcb);
> -     }
>  #ifdef DIAGNOSTIC
> -     else if (p1 != &proc0)
> +     if (p1 != curproc && p1 != &proc0)
>               panic("cpu_fork: curproc");
>  #endif
>       *pcb = p1->p_addr->u_pcb;
> 
> 

Reply via email to