Date: Tue, 25 Aug 2009 17:54:00 GMT From: Stanislav Sedov <stas@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 167794 for review Message-ID: <200908251754.n7PHs0kE092993@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=167794 Change 167794 by stas@stas_yandex on 2009/08/25 17:53:23 - Signal related improvements. Affected files ... .. //depot/projects/valgrind/coregrind/m_sigframe/sigframe-amd64-freebsd.c#6 edit .. //depot/projects/valgrind/coregrind/m_syswrap/syswrap-amd64-freebsd.c#12 edit .. //depot/projects/valgrind/coregrind/pub_core_sigframe.h#5 edit Differences ... ==== //depot/projects/valgrind/coregrind/m_sigframe/sigframe-amd64-freebsd.c#6 (text+ko) ==== @@ -31,8 +31,6 @@ #if defined(VGP_amd64_freebsd) -#warning Needs love! - #include "pub_core_basics.h" #include "pub_core_vki.h" #include "pub_core_threadstate.h" @@ -49,15 +47,6 @@ /* This module creates and removes signal frames for signal deliveries on amd64-freebsd. - - FIXME: sigcontexting is basically broken for the moment. When - delivering a signal, the integer registers and %rflags are - correctly written into the sigcontext, however the FP and SSE state - is not. When returning from a signal, only the integer registers - are restored from the sigcontext; the rest of the CPU state is - restored to what it was before the signal. - - This should be fixed. */ @@ -103,13 +92,33 @@ UInt magicE; }; +struct sigframe +{ + /* Sig handler's return address */ + Addr retaddr; + + Int sigNo; + Addr psigInfo; /* code or pointer to sigContext */ + Addr puContext; /* points to uContext */ + Addr addr; /* "secret" 4th argument */ + Addr phandler; /* "action" or "handler" */ + + /* pointed to by puContext */ + struct vki_ucontext uContext; + + vki_siginfo_t sigInfo; + + struct _vki_fpstate fpstate; + + struct vg_sigframe vg; +}; + /*------------------------------------------------------------*/ /*--- Creating signal frames ---*/ /*------------------------------------------------------------*/ /* Create a plausible-looking sigcontext from the thread's - Vex guest state. NOTE: does not fill in the FP or SSE - bits of sigcontext at the moment. + Vex guest state. */ static void synth_ucontext(ThreadId tid, const vki_siginfo_t *si, @@ -127,8 +136,6 @@ uc->uc_stack = tst->altstack; VG_(memcpy)(&sc->fpstate, fpstate, sizeof(*fpstate)); - // FIXME: save_i387(&tst->arch, fpstate); - # define SC2(reg,REG) sc->reg = tst->arch.vex.guest_##REG SC2(r8,R8); SC2(r9,R9); @@ -146,17 +153,20 @@ SC2(rax,RAX); SC2(rcx,RCX); SC2(rsp,RSP); - +/* + SC2(cs,CS); + SC2(gs,SS); + XXX +*/ SC2(rip,RIP); + sc->addr = (UWord)si->si_addr; + sc->err = err; + sc->fpformat = VKI_FPFMT_NODEV; + sc->len = sizeof(*sc); + sc->ownedfp = VKI_FPOWNED_NONE; sc->rflags = LibVEX_GuestAMD64_get_rflags(&tst->arch.vex); - // FIXME: SC2(cs,CS); - // FIXME: SC2(gs,GS); - // FIXME: SC2(fs,FS); sc->trapno = trapno; - sc->err = err; # undef SC2 - -// sc->cr2 = (UWord)si->_sifields._sigfault._addr; } @@ -224,7 +234,57 @@ frame->magicE = 0x27182818; } +static Addr build_sigframe(ThreadState *tst, + Addr rsp_top_of_frame, + const vki_siginfo_t *siginfo, + const struct vki_ucontext *siguc, + void *handler, UInt flags, + const vki_sigset_t *mask, + void *restorer) +{ + struct sigframe *frame; + Addr rsp = rsp_top_of_frame; + Int sigNo = siginfo->si_signo; + UWord trapno; + UWord err; + + rsp -= sizeof(*frame); + rsp = VG_ROUNDDN(rsp, 16); + frame = (struct sigframe *)rsp; + + if (!extend(tst, rsp, sizeof(*frame))) + return rsp_top_of_frame; + + /* retaddr, siginfo, uContext fields are to be written */ + VG_TRACK( pre_mem_write, Vg_CoreSignal, tst->tid, "signal handler frame", + rsp, offsetof(struct sigframe, vg) ); + + frame->sigNo = sigNo; + frame->retaddr = (Addr)&VG_(amd64_freebsd_SUBST_FOR_sigreturn); + if ((flags & VKI_SA_SIGINFO) == 0) + frame->psigInfo = (Addr)siginfo->si_code; + else + frame->psigInfo = (Addr)&frame->sigInfo; + VG_(memcpy)(&frame->sigInfo, siginfo, sizeof(vki_siginfo_t)); + + trapno = siguc->uc_mcontext.trapno; + err = siguc->uc_mcontext.err; + synth_ucontext(tst->tid, siginfo, trapno, err, mask, + &frame->uContext, &frame->fpstate); + + if (sigNo == VKI_SIGILL && siginfo->si_code > 0) + frame->sigInfo.si_addr = (void*)tst->arch.vex.guest_RIP; + + VG_TRACK( post_mem_write, Vg_CoreSignal, tst->tid, + rsp, offsetof(struct sigframe, vg) ); + + build_vg_sigframe(&frame->vg, tst, mask, flags, sigNo); + + return rsp; +} + + void VG_(sigframe_create)( ThreadId tid, Addr rsp_top_of_frame, const vki_siginfo_t *siginfo, @@ -234,32 +294,26 @@ const vki_sigset_t *mask, void *restorer ) { -// Addr rsp; -// struct sigframe *frame; -// ThreadState* tst = VG_(get_ThreadState)(tid); -// -// rsp = build_vg_sigframe(frame, tst, mask, flags, -// handler, flags, mask, restorer); -// frame = (struct sigframe *)rsp; -// -// /* Set the thread so it will next run the handler. */ -// /* tst->m_rsp = rsp; also notify the tool we've updated RSP */ -// VG_(set_SP)(tid, rsp); -// VG_TRACK( post_reg_write, Vg_CoreSignal, tid, VG_O_STACK_PTR, sizeof(Addr)); -// -// //VG_(printf)("handler = %p\n", handler); - // tst->arch.vex.guest_RIP = (Addr) handler; -// tst->arch.vex.guest_RDI = (ULong) siginfo->si_signo; -// tst->arch.vex.guest_RSI = (Addr) &frame->sigInfo; -// tst->arch.vex.guest_RDX = (Addr) &frame->uContext; -// /* This thread needs to be marked runnable, but we leave that the -// caller to do. */ -// -// if (0) -// VG_(printf)("pushed signal frame; %%RSP now = %#lx, " -// "next %%RIP = %#llx, status=%d\n", -// rsp, tst->arch.vex.guest_RIP, tst->status); -I_die_here; + Addr rsp; + struct sigframe *frame; + ThreadState* tst = VG_(get_ThreadState)(tid); + + rsp = build_sigframe(tst, rsp_top_of_frame, siginfo, siguc, handler, + flags, mask, restorer); + frame = (struct sigframe *)rsp; + + /* Set the thread so it will next run the handler. */ + /* tst->m_rsp = rsp; also notify the tool we've updated RSP */ + VG_(set_SP)(tid, rsp); + VG_TRACK( post_reg_write, Vg_CoreSignal, tid, VG_O_STACK_PTR, sizeof(Addr)); + + //VG_(printf)("handler = %p\n", handler); + tst->arch.vex.guest_RIP = (Addr) handler; + tst->arch.vex.guest_RDI = (ULong) siginfo->si_signo; + tst->arch.vex.guest_RSI = (Addr) &frame->sigInfo; + tst->arch.vex.guest_RDX = (Addr) &frame->uContext; + /* This thread needs to be marked runnable, but we leave that the + caller to do. */ } @@ -296,7 +350,7 @@ static void restore_sigcontext( ThreadState *tst, - struct vki_sigcontext *sc, + struct vki_mcontext *sc, struct _vki_fpstate *fpstate ) { tst->arch.vex.guest_RAX = sc->rax; @@ -315,14 +369,27 @@ tst->arch.vex.guest_R13 = sc->r13; tst->arch.vex.guest_R14 = sc->r14; tst->arch.vex.guest_R15 = sc->r15; -//:: tst->arch.vex.guest_rflags = sc->rflags; +/* + XXX: + tst->arch.vex.guest_rflags = sc->rflags; +*/ tst->arch.vex.guest_RIP = sc->rip; +/* + XXX + tst->arch.vex.guest_CS = sc->cs; + tst->arch.vex.guest_SS = sc->ss; +*/ + VG_(memcpy)(fpstate, &sc->fpstate, sizeof(*fpstate)); +} -//:: tst->arch.vex.guest_CS = sc->cs; -//:: tst->arch.vex.guest_FS = sc->fs; -//:: tst->arch.vex.guest_GS = sc->gs; +static +SizeT restore_sigframe ( ThreadState *tst, + struct sigframe *frame, Int *sigNo ) +{ + if (restore_vg_sigframe(tst, &frame->vg, sigNo)) + restore_sigcontext(tst, &frame->uContext.uc_mcontext, &frame->fpstate); -//:: restore_i387(&tst->arch, fpstate); + return sizeof(*frame); } void VG_(sigframe_destroy)( ThreadId tid ) @@ -337,7 +404,7 @@ /* Correctly reestablish the frame base address. */ rsp = tst->arch.vex.guest_RSP; - size = restore_vg_sigframe(tst, (struct sigframe *)rsp, &sigNo); + size = restore_sigframe(tst, (struct sigframe *)rsp, &sigNo); VG_TRACK( die_mem_stack_signal, rsp - VG_STACK_REDZONE_SZB, size + VG_STACK_REDZONE_SZB ); ==== //depot/projects/valgrind/coregrind/m_syswrap/syswrap-amd64-freebsd.c#12 (text+ko) ==== @@ -1,6 +1,6 @@ /*--------------------------------------------------------------------*/ -/*--- Platform-specific syscalls stuff. syswrap-x86-linux.c ---*/ +/*--- Platform-specific syscalls stuff. syswrap-amd64-freebsd.c ---*/ /*--------------------------------------------------------------------*/ /* @@ -30,16 +30,10 @@ #if defined(VGP_amd64_freebsd) -/* TODO/FIXME jrs 20050207: assignments to the syscall return result - in interrupted_syscall() need to be reviewed. They don't seem - to assign the shadow state. -*/ - #include "pub_core_basics.h" #include "pub_core_vki.h" #include "pub_core_vkiscnums.h" #include "pub_core_threadstate.h" -#include "pub_core_debuginfo.h" // VG_(di_notify_mmap) #include "pub_core_aspacemgr.h" #include "pub_core_debuglog.h" #include "pub_core_libcbase.h" @@ -48,10 +42,9 @@ #include "pub_core_libcproc.h" #include "pub_core_libcsignal.h" #include "pub_core_machine.h" -#include "pub_core_mallocfree.h" #include "pub_core_options.h" #include "pub_core_scheduler.h" -#include "pub_core_sigframe.h" // For VG_(sigframe_destroy)() +#include "pub_core_sigframe.h" #include "pub_core_signals.h" #include "pub_core_syscall.h" #include "pub_core_syswrap.h" @@ -69,7 +62,7 @@ /* Call f(arg1), but first switch stacks, using 'stack' as the new stack, and use 'retaddr' as f's return-to address. Also, clear all - the integer registers before entering f.*/ + the integer registers before entering f. */ __attribute__((noreturn)) void ML_(call_on_new_stack_0_1) ( Addr stack, Addr retaddr, @@ -118,29 +111,12 @@ } /* --------------------------------------------------------------------- - PRE/POST wrappers for x86/Linux-specific syscalls + PRE/POST wrappers for amd64/FreeBSD-specific syscalls ------------------------------------------------------------------ */ #define PRE(name) DEFN_PRE_TEMPLATE(freebsd, name) #define POST(name) DEFN_POST_TEMPLATE(freebsd, name) -#if 0 -struct thr_param { - void (*start_func)(void *); /* thread entry function. */ - void *arg; /* argument for entry function. */ - char *stack_base; /* stack base address. */ - size_t stack_size; /* stack size. */ - char *tls_base; /* tls base address. */ - size_t tls_size; /* tls size. */ - long *child_tid; /* address to store new TID. */ - long *parent_tid; /* parent accesses the new TID here. */ - int flags; /* thread flags. */ - struct rtprio *rtp; /* Real-time scheduling priority */ - void *spare[3]; /* TODO: cpu affinity mask etc. */ -}; -int thr_new(struct thr_param *param, int param_size); -#endif - PRE(sys_thr_new) { static const Bool debug = False; @@ -165,8 +141,8 @@ } VG_(memset)(&tp, 0, sizeof(tp)); VG_(memcpy)(&tp, (void *)ARG1, offsetof(struct vki_thr_param, spare)); - PRE_MEM_WRITE("clone(parent_tidptr)", (Addr)tp.parent_tid, sizeof(long)); - PRE_MEM_WRITE("clone(child_tidptr)", (Addr)tp.child_tid, sizeof(long)); + PRE_MEM_WRITE("thr_new(parent_tidptr)", (Addr)tp.parent_tid, sizeof(long)); + PRE_MEM_WRITE("thr_new(child_tidptr)", (Addr)tp.child_tid, sizeof(long)); VG_(sigfillset)(&blockall); @@ -192,7 +168,7 @@ ctst->arch.vex_shadow1 = ptst->arch.vex_shadow1; ctst->arch.vex_shadow2 = ptst->arch.vex_shadow2; - /* Make sys_clone appear to have returned Success(0) in the + /* Make thr_new appear to have returned Success(0) in the child. */ ctst->arch.vex.guest_RAX = 0; ctst->arch.vex.guest_RDX = 0; @@ -207,8 +183,8 @@ /* Linux has to guess, we don't */ VG_(register_stack)((Addr)tp.stack_base, (Addr)tp.stack_base + tp.stack_size); - /* Assume the clone will succeed, and tell any tool that wants to - know that this thread has come into existence. If the clone + /* Assume the thr_new will succeed, and tell any tool that wants to + know that this thread has come into existence. If the thr_new fails, we'll send out a ll_exit notification for it at the out: label below, to clean up. */ VG_TRACK ( pre_thread_ll_create, tid, ctid ); @@ -223,6 +199,7 @@ /* Set the client state for scheduler to run libthr's trampoline */ ctst->arch.vex.guest_RDI = (Addr)tp.arg; + /* XXX: align on 16-byte boundary? */ ctst->arch.vex.guest_RSP = (Addr)tp.stack_base + tp.stack_size - 8; ctst->arch.vex.guest_RIP = (Addr)tp.start_func; @@ -232,6 +209,10 @@ /* And valgrind's trampoline on its own stack */ stk = ML_(allocstack)(ctid); + if (stk == (Addr)NULL) { + res = VG_(mk_SysRes_Error)( VKI_ENOMEM ); + goto fail; + } tp.stack_base = (void *)ctst->os_state.valgrind_stack_base; tp.stack_size = (Addr)stk - (Addr)tp.stack_base; @@ -240,8 +221,9 @@ VG_(sigprocmask)(VKI_SIG_SETMASK, &savedmask, NULL); +fail: if (sr_isError(res)) { - /* clone failed */ + /* thr_new failed */ VG_(cleanup_thread)(&ctst->arch); ctst->status = VgTs_Empty; /* oops. Better tell the tool the thread exited in a hurry :-) */ @@ -284,13 +266,12 @@ /* Adjust esp to point to start of frame; skip back up over handler ret addr */ tst = VG_(get_ThreadState)(tid); - tst->arch.vex.guest_RSP -= sizeof(Addr); /* QQQ should be redundant */ + tst->arch.vex.guest_RSP -= sizeof(Addr); /* This is only so that the EIP is (might be) useful to report if something goes wrong in the sigreturn */ ML_(fixup_guest_state_to_restart_syscall)(&tst->arch); -// VG_(sigframe_destroy)((struct vki_ucontext *)ARG1, tid, False); VG_(sigframe_destroy)(tid); /* For unclear reasons, it appears we need the syscall to return @@ -299,44 +280,17 @@ driver logic copies it back unchanged. Also, note %EAX is of the guest registers written by VG_(sigframe_destroy). */ rflags = LibVEX_GuestAMD64_get_rflags(&tst->arch.vex); - SET_STATUS_from_SysRes( VG_(mk_SysRes_amd64_freebsd)( tst->arch.vex.guest_RAX, tst->arch.vex.guest_RDX, (rflags & 1) != 0 ? True : False) ); + SET_STATUS_from_SysRes( VG_(mk_SysRes_amd64_freebsd)( tst->arch.vex.guest_RAX, + tst->arch.vex.guest_RDX, (rflags & 1) != 0 ? True : False) ); - /* Check to see if some any signals arose as a result of this. */ - *flags |= SfPollAfter; -} - -#if 0 /* QQQ keep for 6.x signals */ -PRE(sys_rt_sigreturn) -{ - ThreadState* tst; - PRINT("rt_sigreturn ( )"); + /* Tell the driver not to update the guest state with the "result", + and set a bogus result to keep it happy. */ + *flags |= SfNoWriteResult; + SET_STATUS_Success(0); - vg_assert(VG_(is_valid_tid)(tid)); - vg_assert(tid >= 1 && tid < VG_N_THREADS); - vg_assert(VG_(is_running_thread)(tid)); - - /* Adjust esp to point to start of frame; skip back up over handler - ret addr */ - tst = VG_(get_ThreadState)(tid); - tst->arch.vex.guest_ESP -= sizeof(Addr); - - /* This is only so that the EIP is (might be) useful to report if - something goes wrong in the sigreturn */ - ML_(fixup_guest_state_to_restart_syscall)(&tst->arch); - - VG_(sigframe_destroy)(tid, True); - - /* For unclear reasons, it appears we need the syscall to return - without changing %EAX. Since %EAX is the return value, and can - denote either success or failure, we must set up so that the - driver logic copies it back unchanged. Also, note %EAX is of - the guest registers written by VG_(sigframe_destroy). */ - SET_STATUS_from_SysRes( VG_(mk_SysRes_x86_linux)( tst->arch.vex.guest_EAX ) ); - /* Check to see if some any signals arose as a result of this. */ *flags |= SfPollAfter; } -#endif /* This is here because on x86 the off_t is passed in 2 regs. Don't ask about pad. */ ==== //depot/projects/valgrind/coregrind/pub_core_sigframe.h#5 (text+ko) ==== @@ -57,7 +57,6 @@ restore the CPU state from it. */ #ifdef VGO_freebsd extern -#warning DANGER!! void VG_(sigframe_destroy)( ThreadId tid ); #else extern
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908251754.n7PHs0kE092993>