|
From: <sv...@va...> - 2007-03-19 13:38:20
|
Author: sewardj
Date: 2007-03-19 13:38:11 +0000 (Mon, 19 Mar 2007)
New Revision: 6650
Log:
Document and tidy up one of the more arcane corners of signal
handling: why PRE(sys_sigreturn) has to construct a fake syscall
return value which, when written back to the guest state, leaves it
unchanged. It's only taken me about 3 years to realise why :-)
Fixes to ppc platforms to follow.
Modified:
trunk/coregrind/m_syswrap/priv_types_n_macros.h
trunk/coregrind/m_syswrap/syswrap-amd64-linux.c
trunk/coregrind/m_syswrap/syswrap-main.c
trunk/coregrind/m_syswrap/syswrap-x86-linux.c
Modified: trunk/coregrind/m_syswrap/priv_types_n_macros.h
===================================================================
--- trunk/coregrind/m_syswrap/priv_types_n_macros.h 2007-03-14 11:57:37 UTC (rev 6649)
+++ trunk/coregrind/m_syswrap/priv_types_n_macros.h 2007-03-19 13:38:11 UTC (rev 6650)
@@ -94,10 +94,11 @@
SyscallArgLayout;
/* Flags describing syscall wrappers */
-#define SfMayBlock (1 << 1) /* may block */
-#define SfPostOnFail (1 << 2) /* call POST() function on failure */
-#define SfPollAfter (1 << 3) /* poll for signals on completion */
-#define SfYieldAfter (1 << 4) /* yield on completion */
+#define SfMayBlock (1 << 1) /* may block */
+#define SfPostOnFail (1 << 2) /* call POST() function on failure */
+#define SfPollAfter (1 << 3) /* poll for signals on completion */
+#define SfYieldAfter (1 << 4) /* yield on completion */
+#define SfNoWriteResult (1 << 5) /* don't write result to guest state */
/* ---------------------------------------------------------------------
@@ -312,14 +313,7 @@
status->sres = (zzz); \
} while (0)
-/* A lamentable kludge */
-#define SET_STATUS_Failure_NO_SANITY_CHECK(zzz) \
- do { Word wzz = (Word)(zzz); \
- status->what = SsFailure; \
- status->val = wzz; \
- } while (0)
-
#define PRINT(format, args...) \
if (VG_(clo_trace_syscalls)) \
VG_(printf)(format, ## args)
Modified: trunk/coregrind/m_syswrap/syswrap-amd64-linux.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-amd64-linux.c 2007-03-14 11:57:37 UTC (rev 6649)
+++ trunk/coregrind/m_syswrap/syswrap-amd64-linux.c 2007-03-19 13:38:11 UTC (rev 6650)
@@ -451,30 +451,41 @@
PRE(sys_rt_sigreturn)
{
+ /* This isn't really a syscall at all - it's a misuse of the
+ syscall mechanism by m_sigframe. VG_(sigframe_create) sets the
+ return address of the signal frames it creates to be a short
+ piece of code which does this "syscall". The only purpose of
+ the syscall is to call VG_(sigframe_destroy), which restores the
+ thread's registers from the frame and then removes it.
+ Consequently we must ask the syswrap driver logic not to write
+ back the syscall "result" as that would overwrite the
+ just-restored register state. */
+
ThreadState* tst;
- PRINT("rt_sigreturn ( )");
+ PRINT("sys_rt_sigreturn ( )");
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
+ /* Adjust RSP 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);
/* This is only so that the RIP is (might be) useful to report if
- something goes wrong in the sigreturn */
+ something goes wrong in the sigreturn. JRS 20070318: no idea
+ what this is for */
ML_(fixup_guest_state_to_restart_syscall)(&tst->arch);
+ /* Restore register state from frame and remove it, as
+ described above */
VG_(sigframe_destroy)(tid, True);
- /* For unclear reasons, it appears we need the syscall to return
- without changing %RAX. Since %RAX 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 %RAX is of
- the guest registers written by VG_(sigframe_destroy). */
- SET_STATUS_from_SysRes( VG_(mk_SysRes_amd64_linux)( tst->arch.vex.guest_RAX ) );
+ /* 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);
/* Check to see if some any signals arose as a result of this. */
*flags |= SfPollAfter;
Modified: trunk/coregrind/m_syswrap/syswrap-main.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-main.c 2007-03-14 11:57:37 UTC (rev 6649)
+++ trunk/coregrind/m_syswrap/syswrap-main.c 2007-03-19 13:38:11 UTC (rev 6650)
@@ -858,12 +858,17 @@
if (sci->status.what == SsComplete && !sci->status.sres.isError) {
/* The pre-handler completed the syscall itself, declaring
success. */
- PRINT(" --> [pre-success] Success(0x%llx)\n", (ULong)sci->status.sres.res );
-
+ if (sci->flags & SfNoWriteResult) {
+ PRINT(" --> [pre-success] NoWriteResult\n");
+ } else {
+ PRINT(" --> [pre-success] Success(0x%llx)\n",
+ (ULong)sci->status.sres.res );
+ }
/* In this case the allowable flags are to ask for a signal-poll
and/or a yield after the call. Changing the args isn't
allowed. */
- vg_assert(0 == (sci->flags & ~(SfPollAfter | SfYieldAfter)));
+ vg_assert(0 == (sci->flags
+ & ~(SfPollAfter | SfYieldAfter | SfNoWriteResult)));
vg_assert(eq_SyscallArgs(&sci->args, &sci->orig_args));
}
@@ -971,7 +976,8 @@
/* Dump the syscall result back in the guest state. This is
a platform-specific action. */
- putSyscallStatusIntoGuestState( &sci->status, &tst->arch.vex );
+ if (!(sci->flags & SfNoWriteResult))
+ putSyscallStatusIntoGuestState( &sci->status, &tst->arch.vex );
/* Situation now:
- the guest state is now correctly modified following the syscall
@@ -1022,11 +1028,13 @@
/* Validate current syscallInfo entry. In particular we require
that the current .status matches what's actually in the guest
- state. */
+ state. At least in the normal case where we have actually
+ previously written the result into the guest state. */
vg_assert(sci->status.what == SsComplete);
getSyscallStatusFromGuestState( &test_status, &tst->arch.vex );
- vg_assert(eq_SyscallStatus( &sci->status, &test_status ));
+ if (!(sci->flags & SfNoWriteResult))
+ vg_assert(eq_SyscallStatus( &sci->status, &test_status ));
/* Ok, looks sane */
/* Get the system call number. Because the pre-handler isn't
@@ -1061,7 +1069,8 @@
post-handler for sys_open can change the result from success to
failure if the kernel supplied a fd that it doesn't like), once
again dump the syscall result back in the guest state.*/
- putSyscallStatusIntoGuestState( &sci->status, &tst->arch.vex );
+ if (!(sci->flags & SfNoWriteResult))
+ putSyscallStatusIntoGuestState( &sci->status, &tst->arch.vex );
/* Do any post-syscall actions required by the tool. */
if (VG_(needs).syscall_wrapper)
@@ -1314,7 +1323,8 @@
canonical = convert_SysRes_to_SyscallStatus(
VG_(mk_SysRes_Error)( VKI_EINTR )
);
- putSyscallStatusIntoGuestState( &canonical, &th_regs->vex );
+ if (!(sci->flags & SfNoWriteResult))
+ putSyscallStatusIntoGuestState( &canonical, &th_regs->vex );
sci->status = canonical;
VG_(post_syscall)(tid);
}
@@ -1328,7 +1338,8 @@
if (debug)
VG_(printf)(" completed\n");
canonical = convert_SysRes_to_SyscallStatus( sres );
- putSyscallStatusIntoGuestState( &canonical, &th_regs->vex );
+ if (!(sci->flags & SfNoWriteResult))
+ putSyscallStatusIntoGuestState( &canonical, &th_regs->vex );
sci->status = canonical;
VG_(post_syscall)(tid);
}
Modified: trunk/coregrind/m_syswrap/syswrap-x86-linux.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-x86-linux.c 2007-03-14 11:57:37 UTC (rev 6649)
+++ trunk/coregrind/m_syswrap/syswrap-x86-linux.c 2007-03-19 13:38:11 UTC (rev 6650)
@@ -935,8 +935,11 @@
PRE(sys_sigreturn)
{
+ /* See comments on PRE(sys_rt_sigreturn) in syswrap-amd64-linux.c for
+ an explanation of what follows. */
+
ThreadState* tst;
- PRINT("sigreturn ( )");
+ PRINT("sys_sigreturn ( )");
vg_assert(VG_(is_valid_tid)(tid));
vg_assert(tid >= 1 && tid < VG_N_THREADS);
@@ -946,19 +949,19 @@
sigreturn sequence's "popl %eax" and handler ret addr */
tst = VG_(get_ThreadState)(tid);
tst->arch.vex.guest_ESP -= sizeof(Addr)+sizeof(Word);
+ /* XXX why does ESP change differ from rt_sigreturn case below? */
/* 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);
+ /* Restore register state from frame and remove it */
VG_(sigframe_destroy)(tid, False);
- /* 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 ) );
+ /* 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);
/* Check to see if some any signals arose as a result of this. */
*flags |= SfPollAfter;
@@ -966,8 +969,11 @@
PRE(sys_rt_sigreturn)
{
+ /* See comments on PRE(sys_rt_sigreturn) in syswrap-amd64-linux.c for
+ an explanation of what follows. */
+
ThreadState* tst;
- PRINT("rt_sigreturn ( )");
+ PRINT("sys_rt_sigreturn ( )");
vg_assert(VG_(is_valid_tid)(tid));
vg_assert(tid >= 1 && tid < VG_N_THREADS);
@@ -977,19 +983,19 @@
ret addr */
tst = VG_(get_ThreadState)(tid);
tst->arch.vex.guest_ESP -= sizeof(Addr);
+ /* XXX why does ESP change differ from sigreturn case above? */
/* 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);
+ /* Restore register state from frame and remove it */
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 ) );
+ /* 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);
/* Check to see if some any signals arose as a result of this. */
*flags |= SfPollAfter;
|