|
From: <sv...@va...> - 2009-03-22 11:18:13
|
Author: sewardj
Date: 2009-03-22 11:18:03 +0000 (Sun, 22 Mar 2009)
New Revision: 9485
Log:
On Darwin, it is essential to use _nocancel versions of syscalls
wherever possible, for reasons discussed at length in
Darwin-notes.txt.
Modified:
branches/DARWIN/coregrind/m_debuglog.c
branches/DARWIN/coregrind/m_libcfile.c
branches/DARWIN/coregrind/m_libcproc.c
branches/DARWIN/coregrind/m_libcsignal.c
branches/DARWIN/docs/internals/Darwin-notes.txt
Modified: branches/DARWIN/coregrind/m_debuglog.c
===================================================================
--- branches/DARWIN/coregrind/m_debuglog.c 2009-03-22 10:14:39 UTC (rev 9484)
+++ branches/DARWIN/coregrind/m_debuglog.c 2009-03-22 11:18:03 UTC (rev 9485)
@@ -46,6 +46,11 @@
/* This module is also notable because it is linked into both
stage1 and stage2. */
+/* IMPORTANT: on Darwin it is essential to use the _nocancel versions
+ of syscalls rather than the vanilla version, if a _nocancel version
+ is available. See docs/internals/Darwin-notes.txt for the reason
+ why. */
+
#include "pub_core_basics.h" /* basic types */
#include "pub_core_vkiscnums.h" /* for syscall numbers */
#include "pub_core_debuglog.h" /* our own iface */
@@ -395,6 +400,10 @@
#elif defined(VGP_x86_darwin)
+/* Using _SYSNO_INDEX rather than _SYSNO_NUM assumes that these are
+ Unix-class syscalls (which they are). Unfortunately _SYSNO_NUM
+ involves a C-style "cond ? :" expression which doesn't impress the
+ Darwin assembler very much. */
__attribute__((noinline))
static UInt local_sys_write_stderr ( HChar* buf, Int n )
{
@@ -406,7 +415,8 @@
"pushl %%eax\n"
"movl $1, %%eax\n" /* push stderr */
"pushl %%eax\n"
- "movl $4, %%eax\n" /* %eax = __NR_write */
+ "movl $"VG_STRINGIFY(VG_DARWIN_SYSNO_INDEX(__NR_write_nocancel))
+ ", %%eax\n"
"pushl %%eax\n" /* push fake return address */
"int $0x80\n" /* write(stderr, buf, n) */
"jnc 1f\n" /* jump if no error */
@@ -425,7 +435,7 @@
{
UInt __res;
__asm__ volatile (
- "movl $20, %%eax\n" /* set %eax = __NR_getpid */
+ "movl $"VG_STRINGIFY(VG_DARWIN_SYSNO_INDEX(__NR_getpid))", %%eax\n"
"int $0x80\n" /* getpid() */
"movl %%eax, %0\n" /* set __res = eax */
: "=mr" (__res)
@@ -444,7 +454,8 @@
"movq $1, %%rdi\n" /* push stderr */
"movq %1, %%rsi\n" /* push buf */
"movl %2, %%edx\n" /* push n */
- "movl $" VG_STRINGIFY(VG_DARWIN_SYSNO_NUM(__NR_write)) ", %%eax\n"
+ "movl $"VG_STRINGIFY(VG_DARWIN_SYSNO_NUM(__NR_write_nocancel))
+ ", %%eax\n"
"syscall\n" /* write(stderr, buf, n) */
"jnc 1f\n" /* jump if no error */
"movq $-1, %%rax\n" /* return -1 if error */
@@ -460,7 +471,7 @@
{
UInt __res;
__asm__ volatile (
- "movl $" VG_STRINGIFY(VG_DARWIN_SYSNO_NUM(__NR_getpid)) ", %%eax\n"
+ "movl $"VG_STRINGIFY(VG_DARWIN_SYSNO_NUM(__NR_getpid))", %%eax\n"
"syscall\n" /* getpid() */
"movl %%eax, %0\n" /* set __res = eax */
: "=mr" (__res)
Modified: branches/DARWIN/coregrind/m_libcfile.c
===================================================================
--- branches/DARWIN/coregrind/m_libcfile.c 2009-03-22 10:14:39 UTC (rev 9484)
+++ branches/DARWIN/coregrind/m_libcfile.c 2009-03-22 11:18:03 UTC (rev 9485)
@@ -41,6 +41,11 @@
#include "pub_core_clientstate.h" // VG_(fd_hard_limit)
#include "pub_core_syscall.h"
+/* IMPORTANT: on Darwin it is essential to use the _nocancel versions
+ of syscalls rather than the vanilla version, if a _nocancel version
+ is available. See docs/internals/Darwin-notes.txt for the reason
+ why. */
+
/* ---------------------------------------------------------------------
File stuff
------------------------------------------------------------------ */
@@ -103,19 +108,20 @@
SysRes VG_(open) ( const Char* pathname, Int flags, Int mode )
{
- SysRes res = VG_(do_syscall3)(__NR_open, (UWord)pathname, flags, mode);
+ SysRes res = VG_(do_syscall3)(__NR_open_nocancel,
+ (UWord)pathname, flags, mode);
return res;
}
void VG_(close) ( Int fd )
{
- (void)VG_(do_syscall1)(__NR_close, fd);
+ (void)VG_(do_syscall1)(__NR_close_nocancel, fd);
}
Int VG_(read) ( Int fd, void* buf, Int count)
{
Int ret;
- SysRes res = VG_(do_syscall3)(__NR_read, fd, (UWord)buf, count);
+ SysRes res = VG_(do_syscall3)(__NR_read_nocancel, fd, (UWord)buf, count);
if (sr_isError(res)) {
ret = - (Int)(Word)sr_Err(res);
vg_assert(ret < 0);
@@ -129,7 +135,7 @@
Int VG_(write) ( Int fd, const void* buf, Int count)
{
Int ret;
- SysRes res = VG_(do_syscall3)(__NR_write, fd, (UWord)buf, count);
+ SysRes res = VG_(do_syscall3)(__NR_write_nocancel, fd, (UWord)buf, count);
if (sr_isError(res)) {
ret = - (Int)(Word)sr_Err(res);
vg_assert(ret < 0);
@@ -143,8 +149,9 @@
Int VG_(select) ( Int nfds, void *rfds, void *wfds, void *efds, void *timeout)
{
Int ret;
- SysRes res = VG_(do_syscall5)(__NR_select, nfds, (Addr)rfds, (Addr)wfds,
- (Addr)efds, (Addr)timeout);
+ SysRes res = VG_(do_syscall5)(__NR_select_nocancel, nfds,
+ (Addr)rfds, (Addr)wfds,
+ (Addr)efds, (Addr)timeout);
if (sr_isError(res)) {
ret = - (Int)(Word)sr_Err(res);
vg_assert(ret < 0);
@@ -343,7 +350,7 @@
/* Returns -1 on error. */
Int VG_(fcntl) ( Int fd, Int cmd, Addr arg )
{
- SysRes res = VG_(do_syscall3)(__NR_fcntl, fd, cmd, arg);
+ SysRes res = VG_(do_syscall3)(__NR_fcntl_nocancel, fd, cmd, arg);
return sr_isError(res) ? -1 : sr_Res(res);
}
@@ -554,11 +561,11 @@
{
SysRes res;
# if defined(VGP_amd64_darwin)
- res = VG_(do_syscall4)(__NR_pread, fd, (UWord)buf, count, offset);
+ res = VG_(do_syscall4)(__NR_pread_nocancel, fd, (UWord)buf, count, offset);
return res;
# elif defined(VGP_x86_darwin)
/* ppc32-darwin is the same, but with the args inverted */
- res = VG_(do_syscall5)(__NR_pread, fd, (UWord)buf, count,
+ res = VG_(do_syscall5)(__NR_pread_nocancel, fd, (UWord)buf, count,
offset & 0xffffffff, offset >> 32);
return res;
# elif defined(VGO_linux) || defined(VGO_aix5)
@@ -863,7 +870,7 @@
# elif defined(VGO_darwin)
SysRes res;
- res = VG_(do_syscall3)(__NR_accept, sock, (UWord)addr, (UWord)len);
+ res = VG_(do_syscall3)(__NR_accept_nocancel, sock, (UWord)addr, (UWord)len);
return sr_isError(res) ? -1 : sr_Res(res);
# else
@@ -896,7 +903,8 @@
# elif defined(VGO_darwin)
SysRes res;
- res = VG_(do_syscall3)(__NR_connect, sockfd, (UWord)serv_addr, addrlen);
+ res = VG_(do_syscall3)(__NR_connect_nocancel,
+ sockfd, (UWord)serv_addr, addrlen);
return sr_isError(res) ? -1 : sr_Res(res);
# else
@@ -937,7 +945,7 @@
# elif defined(VGP_x86_darwin) || defined(VGP_amd64_darwin)
SysRes res;
- res = VG_(do_syscall3)(__NR_write, sd, (UWord)msg, count);
+ res = VG_(do_syscall3)(__NR_write_nocancel, sd, (UWord)msg, count);
return sr_isError(res) ? -1 : sr_Res(res);
# else
Modified: branches/DARWIN/coregrind/m_libcproc.c
===================================================================
--- branches/DARWIN/coregrind/m_libcproc.c 2009-03-22 10:14:39 UTC (rev 9484)
+++ branches/DARWIN/coregrind/m_libcproc.c 2009-03-22 11:18:03 UTC (rev 9485)
@@ -49,6 +49,10 @@
/* --- !!! --- EXTERNAL HEADERS end --- !!! --- */
#endif
+/* IMPORTANT: on Darwin it is essential to use the _nocancel versions
+ of syscalls rather than the vanilla version, if a _nocancel version
+ is available. See docs/internals/Darwin-notes.txt for the reason
+ why. */
/* ---------------------------------------------------------------------
Command line and environment stuff
@@ -254,7 +258,8 @@
Int VG_(waitpid)(Int pid, Int *status, Int options)
{
# if defined(VGO_linux) || defined(VGO_darwin)
- SysRes res = VG_(do_syscall4)(__NR_wait4, pid, (UWord)status, options, 0);
+ SysRes res = VG_(do_syscall4)(__NR_wait4_nocancel,
+ pid, (UWord)status, options, 0);
return sr_isError(res) ? -1 : sr_Res(res);
# elif defined(VGO_aix5)
/* magic number 4 obtained by truss-ing a C program doing
Modified: branches/DARWIN/coregrind/m_libcsignal.c
===================================================================
--- branches/DARWIN/coregrind/m_libcsignal.c 2009-03-22 10:14:39 UTC (rev 9484)
+++ branches/DARWIN/coregrind/m_libcsignal.c 2009-03-22 11:18:03 UTC (rev 9485)
@@ -37,6 +37,11 @@
#include "pub_core_syscall.h"
#include "pub_core_libcsignal.h" /* self */
+/* IMPORTANT: on Darwin it is essential to use the _nocancel versions
+ of syscalls rather than the vanilla version, if a _nocancel version
+ is available. See docs/internals/Darwin-notes.txt for the reason
+ why. */
+
/* sigemptyset, sigfullset, sigaddset and sigdelset return 0 on
success and -1 on error. */
/* I believe the indexing scheme in ->sig[] is also correct for
Modified: branches/DARWIN/docs/internals/Darwin-notes.txt
===================================================================
--- branches/DARWIN/docs/internals/Darwin-notes.txt 2009-03-22 10:14:39 UTC (rev 9484)
+++ branches/DARWIN/docs/internals/Darwin-notes.txt 2009-03-22 11:18:03 UTC (rev 9485)
@@ -1,6 +1,53 @@
-Valgrind-developer notes, todos re the MacOSX port.
+Valgrind-developer notes, re the MacOSX port
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+JRS 22 Mar 09: re these comments in m_libc* and m_debuglog:
+
+/* IMPORTANT: on Darwin it is essential to use the _nocancel versions
+ of syscalls rather than the vanilla version, if a _nocancel version
+ is available. See docs/internals/Darwin-notes.txt for the reason
+ why. */
+
+when Valgrind does (for its own purposes, not for the client)
+read/write/open/close etc syscalls, it really is critical to use the
+_nocancel versions of syscalls rather than the vanilla versions. This
+holds throughout the entire code base: whenever V does a syscall for
+its own purposes, we must use the _nocancel version if it exists.
+This is of course most prevalent in m_libc* since all of our
+own-purpose (non-client) syscalls should get routed through there.
+
+Why? Because on Darwin, pthread cancellation is done within the
+kernel (unlike on Linux, iiuc). And read/write/open/close and a whole
+bunch of other syscalls to do with stream I/O are cancellation points.
+So what can happen is, client informs the kernel that a given thread
+is to be cancelled. Then at the next (eg) VG_(printf) call by that
+thread, which leads to a sys_write, the write syscall gets hit by the
+cancellation request, and is duly nuked by the kernel. Of course from
+the outside it looks as if the thread had mysteriously disappeared off
+the radar for no reason.
+
+In short, we need to use _nocancel versions in order to ensure that
+cancellation requests only take effect at the places where the client
+does a syscall, and not the places where Valgrind does syscalls.
+
+How observed: using the standard pipe-based implementation in
+coregrind/m_scheduler/sema.c, none/tests/pth_cancel1 would hang
+(compared to succeeding using native Darwin semaphores). And if the
+"pause()" call in said test is turned into a spin ("while (1) ;") then
+the entire Valgrind run mysteriously disappears, rather than spinning
+using native Darwin semaphores.
+
+Because the pipe-based semaphore intensively uses sys_read/sys_write,
+it is not surprising that it inadvertantly was eating up cancellation
+requests directed to client threads. With abovementioned change in
+force the pipe-based semaphore appears to work correctly.
+
+
+
+Valgrind-developer notes, todos re the MacOSX port
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
* m_syswrap/syscall-amd64-darwin.S
- correct signal mask is not applied during syscall
- restart-labels are completely bogus
|