p4-projects@freebsd.org
[Top] [All Lists]

PERFORCE change 167794 for review

Subject: PERFORCE change 167794 for review
From: Stanislav Sedov
Date: Tue, 25 Aug 2009 17:54:00 GMT
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 
_______________________________________________
p4-projects@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/p4-projects
To unsubscribe, send any mail to "p4-projects-unsubscribe@xxxxxxxxxxx"

<Prev in Thread] Current Thread [Next in Thread>
  • PERFORCE change 167794 for review, Stanislav Sedov <=