diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index c1b582c2b..90920204f 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -1601,12 +1601,23 @@ PRE(sys_munlockall) PRE_REG_READ0(long, "munlockall"); } -// XXX: sort of x86/Linux-specific +// This has different signatures for different platforms. +// +// x86: int sys_pipe(unsigned long __user *fildes); +// AMD64: long sys_pipe(int *fildes); +// ppc32: int sys_pipe(int __user *fildes); +// ppc64: int sys_pipe(int __user *fildes); +// +// The type of the argument is most important, and it is an array of 32 bit +// values in all cases. (The return type differs across platforms, but it +// is not used.) So we use 'int' as its type. This fixed bug #113230 which +// was caused by using an array of 'unsigned long's, which didn't work on +// AMD64. PRE(sys_pipe) { PRINT("sys_pipe ( %p )", ARG1); - PRE_REG_READ1(int, "pipe", unsigned long *, filedes); - PRE_MEM_WRITE( "pipe(filedes)", ARG1, 2*sizeof(long) ); + PRE_REG_READ1(int, "pipe", int *, filedes); + PRE_MEM_WRITE( "pipe(filedes)", ARG1, 2*sizeof(int) ); } POST(sys_pipe) { diff --git a/docs/internals/3_0_BUGSTATUS.txt b/docs/internals/3_0_BUGSTATUS.txt index 59e4122c5..ae66e6780 100644 --- a/docs/internals/3_0_BUGSTATUS.txt +++ b/docs/internals/3_0_BUGSTATUS.txt @@ -155,6 +155,13 @@ FIXED-30BRANCH: TODO FIXED-TRUNK: TODO FIXED-30BRANCH: TODO +---------------------------------------------------------------- +113230 Valgrind sys_pipe on x86-64 wrongly thinks file descriptors + should be 64bit + +FIXED-TRUNK: vg:4669 +FIXED-30BRANCH: TODO + ======================================================================== === Bugs targeted for 3.1.0 and 3.0.1 (all done, 3.0.1 released) === diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 28bc93060..7a01b4569 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -63,6 +63,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \ oset_test.stderr.exp oset_test.stdout.exp oset_test.vgtest \ partiallydefinedeq.vgtest partiallydefinedeq.stderr.exp \ partiallydefinedeq.stdout.exp \ + pipe.stderr.exp pipe.vgtest \ pointer-trace.vgtest \ pointer-trace.stderr.exp pointer-trace.stderr.exp64 \ post-syscall.stderr.exp post-syscall.stdout.exp post-syscall.vgtest \ @@ -107,7 +108,7 @@ check_PROGRAMS = \ nanoleak new_nothrow \ null_socket oset_test overlap \ partiallydefinedeq \ - pointer-trace \ + pipe pointer-trace \ post-syscall \ realloc1 realloc2 realloc3 \ sigaltstack signal2 sigprocmask sigkill \ diff --git a/memcheck/tests/pipe.c b/memcheck/tests/pipe.c new file mode 100644 index 000000000..cf97739c8 --- /dev/null +++ b/memcheck/tests/pipe.c @@ -0,0 +1,16 @@ +// This is a regtest for bug #113230, in which 64-bit platforms mistakenly +// behaved as if pipe() took an array of 64-bit ints, when it really takes +// an array of 32-bit ints. + +#include +#include +#include + +int main(int argc, char *argv[]) +{ + int *filedes = malloc(2 * sizeof(int)); + + pipe(filedes); + + return 0; +} diff --git a/memcheck/tests/pipe.stderr.exp b/memcheck/tests/pipe.stderr.exp new file mode 100644 index 000000000..e69de29bb diff --git a/memcheck/tests/pipe.vgtest b/memcheck/tests/pipe.vgtest new file mode 100644 index 000000000..45698a44f --- /dev/null +++ b/memcheck/tests/pipe.vgtest @@ -0,0 +1,2 @@ +prog: pipe +vgopts: -q