|
From: Bart V. A. <bar...@gm...> - 2009-08-27 17:36:31
|
Hello, If there are no objections I will commit the changes below on the 3.5 branch. These changes fix https://bugs.kde.org/show_bug.cgi?id=204843 and also add a regression test for Valgrind's handling of /proc accesses. I have verified that the output of "make regtest" is the same as for the trunk. I'm posting the changes here first because there was some discussion about the proper approach to fix this issue. $ svn co svn://svn.valgrind.org/valgrind/branches/VALGRIND_3_5_BRANCHvalgrind-3.5.x $ cd valgrind-3.5.x $ svn merge -r10859:10860 svn://svn.valgrind.org/valgrind/trunk $ svn merge -r10862:10868 svn://svn.valgrind.org/valgrind/trunk $ svn merge -r10870:10873 svn://svn.valgrind.org/valgrind/trunk $ svn diff Property changes on: . ___________________________________________________________________ Deleted: svn:mergeinfo Index: configure.in =================================================================== --- configure.in (revision 10874) +++ configure.in (working copy) @@ -1470,14 +1470,6 @@ #---------------------------------------------------------------------------- -# Check for /proc filesystem -#---------------------------------------------------------------------------- -AC_CHECK_FILES(/proc/self/fd /proc/self/exe /proc/self/maps, - [ AC_DEFINE([HAVE_PROC], 1, [can use /proc filesystem]) ], - []) - - -#---------------------------------------------------------------------------- # Checks for C header files. #---------------------------------------------------------------------------- @@ -1534,6 +1526,7 @@ pthread_rwlock_timedrdlock \ pthread_rwlock_timedwrlock \ pthread_spin_lock \ + readlinkat \ semtimedop \ signalfd \ sigwaitinfo \ Property changes on: none/tests ___________________________________________________________________ Modified: svn:ignore - *.dSYM *.so *.stderr.diff* *.stderr.out *.stdout.diff* *.stdout.out .deps ansi args async-sigs as_mmap as_shm bitfield1 blockfault bug129866 closeall coolo_sigaction coolo_strlen discard exec-sigmask execve faultstatus fcntl_setown fdleak_cmsg fdleak_creat fdleak_dup fdleak_dup2 fdleak_fcntl fdleak_ipv4 fdleak_open fdleak_pipe fdleak_socketpair floored fork fucomip gxx304 insn_basic insn_basic.c insn_cmov insn_cmov.c insn_fpu insn_fpu.c insn_mmx insn_mmx.c insn_mmxext insn_mmxext.c insn_sse insn_sse.c insn_sse2 insn_sse2.c Makefile Makefile.in manythreads map_unaligned map_unmap mq mremap mremap2 munmap_exe nestedfns pending pluto pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple pth_detached pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock pth_semaphore1 pth_simple_mutex pth_simple_threads pth_specific pth_stackalign pth_yield rcrl readline1 resolv res_search rlimit_nofile selfrun sem semlimit sha1_test shortpush shorts sigstackgrowth smc1 stackgrowth susphello syscall-restart1 syscall-restart2 syslog system thread-exits threaded-fork threadederrno timestamp tls vgcore.* vgprintf yield + *.dSYM *.so *.stderr.diff* *.stderr.out *.stdout.diff* *.stdout.out .deps ansi args async-sigs as_mmap as_shm bitfield1 blockfault bug129866 closeall coolo_sigaction coolo_strlen discard exec-sigmask execve faultstatus fcntl_setown fdleak_cmsg fdleak_creat fdleak_dup fdleak_dup2 fdleak_fcntl fdleak_ipv4 fdleak_open fdleak_pipe fdleak_socketpair floored fork fucomip gxx304 insn_basic insn_basic.c insn_cmov insn_cmov.c insn_fpu insn_fpu.c insn_mmx insn_mmx.c insn_mmxext insn_mmxext.c insn_sse insn_sse.c insn_sse2 insn_sse2.c Makefile Makefile.in manythreads map_unaligned map_unmap mq mremap mremap2 munmap_exe nestedfns pending pluto procfs-cmdline-exe pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple pth_detached pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock pth_semaphore1 pth_simple_mutex pth_simple_threads pth_specific pth_stackalign pth_yield rcrl readline1 resolv res_search rlimit_nofile selfrun sem semlimit sha1_test shortpush shorts sigstackgrowth smc1 stackgrowth susphello syscall-restart1 syscall-restart2 syslog system thread-exits threaded-fork threadederrno timestamp tls vgcore.* vgprintf yield Index: none/tests/Makefile.am =================================================================== --- none/tests/Makefile.am (revision 10874) +++ none/tests/Makefile.am (working copy) @@ -88,6 +88,10 @@ munmap_exe.stderr.exp munmap_exe.vgtest \ nestedfns.stderr.exp nestedfns.stdout.exp nestedfns.vgtest \ pending.stdout.exp pending.stderr.exp pending.vgtest \ + procfs-linux.stderr.exp-with-readlinkat \ + procfs-linux.stderr.exp-without-readlinkat \ + procfs-linux.vgtest \ + procfs-non-linux.stderr.exp procfs-non-linux.vgtest \ pth_atfork1.stderr.exp pth_atfork1.stdout.exp pth_atfork1.vgtest \ pth_blockedsig.stderr.exp \ pth_blockedsig.stdout.exp pth_blockedsig.vgtest \ @@ -155,6 +159,7 @@ munmap_exe map_unaligned map_unmap mq \ nestedfns \ pending \ + procfs-cmdline-exe \ pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple \ pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock \ pth_stackalign \ Index: coregrind/m_syswrap/syswrap-generic.c =================================================================== --- coregrind/m_syswrap/syswrap-generic.c (revision 10874) +++ coregrind/m_syswrap/syswrap-generic.c (working copy) @@ -3526,7 +3526,7 @@ } PRE_MEM_RASCIIZ( "open(filename)", ARG1 ); -#if HAVE_PROC +#if defined(VGO_linux) /* Handle the case where the open is of /proc/self/cmdline or /proc/<pid>/cmdline, and just give it a copy of the fd for the fake file we cooked up at startup (in m_main). Also, seek the @@ -3551,7 +3551,7 @@ return; } } -#endif // HAVE_PROC +#endif // defined(VGO_linux) /* Otherwise handle normally */ *flags |= SfMayBlock; @@ -3674,7 +3674,7 @@ PRE_MEM_WRITE( "readlink(buf)", ARG2,ARG3 ); { -#if HAVE_PROC +#if defined(VGO_linux) /* * Handle the case where readlink is looking at /proc/self/exe or * /proc/<pid>/exe. @@ -3690,7 +3690,7 @@ SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, (UWord)name, ARG2, ARG3)); } else -#endif // HAVE_PROC +#endif // defined(VGO_linux) { /* Normal case */ SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, ARG1, ARG2, ARG3)); Index: coregrind/m_aspacemgr/aspacemgr-linux.c =================================================================== --- coregrind/m_aspacemgr/aspacemgr-linux.c (revision 10874) +++ coregrind/m_aspacemgr/aspacemgr-linux.c (working copy) @@ -2986,7 +2986,7 @@ #endif // HAVE_MREMAP -#if HAVE_PROC +#if defined(VGO_linux) /*-----------------------------------------------------------------*/ /*--- ---*/ @@ -3493,7 +3493,7 @@ return !css_overflowed; } -#endif // HAVE_PROC +#endif // defined(VGO_linux) #endif // defined(VGO_linux) || defined(VGO_darwin) Index: coregrind/m_main.c =================================================================== --- coregrind/m_main.c (revision 10874) +++ coregrind/m_main.c (working copy) @@ -1802,7 +1802,7 @@ // when it tries to open /proc/<pid>/cmdline for itself. // p: setup file descriptors //-------------------------------------------------------------- -#if !HAVE_PROC +#if !defined(VGO_linux) // client shouldn't be using /proc! VG_(cl_cmdline_fd) = -1; #else |
|
From: Julian S. <js...@ac...> - 2009-08-27 18:38:49
|
> If there are no objections I will commit the changes below on the 3.5 > branch. I'd like to study this a bit more before that happens. I'll look at it and get back to you by this time tomorrow. J |
|
From: Bart V. A. <bar...@gm...> - 2009-08-27 18:41:25
|
On Thu, Aug 27, 2009 at 8:38 PM, Julian Seward <js...@ac...> wrote: > > If there are no objections I will commit the changes below on the 3.5 > > branch. > > I'd like to study this a bit more before that happens. I'll look at it > and get back to you by this time tomorrow. Please note that the source code of the newly added regression tests is missing in the output of the "svn diff" command. It's not clear to me whether this is a Subversion bug or feature. The full set of changes can be reproduced using the merge commands I posted. Bart. |
|
From: Julian S. <js...@ac...> - 2009-08-28 15:19:41
|
Hi Bart,
Ok for the merge, but please only merge the absolute minimal set
of changes needed to fix the bug on the branch:
* removal of # Check for /proc filesystem
* changes in coregrind/m_syswrap/syswrap-generic.c,
coregrind/m_aspacemgr/aspacemgr-linux.c,
coregrind/m_main.c
Don't merge the new regression tests nor the svn:ignore changes.
Obviously it's good that the bug is fixed on the branch. However
I don't want to have to re-verify that there are no regtest build
problems on the branch. Hence the minimal change set only.
J
On Thursday 27 August 2009 07:36:18 pm Bart Van Assche wrote:
> Hello,
>
> If there are no objections I will commit the changes below on the 3.5
> branch. These changes fix https://bugs.kde.org/show_bug.cgi?id=204843 and
> also add a regression test for Valgrind's handling of /proc accesses. I
> have verified that the output of "make regtest" is the same as for the
> trunk. I'm posting the changes here first because there was some discussion
> about the proper approach to fix this issue.
>
> $ svn co
> svn://svn.valgrind.org/valgrind/branches/VALGRIND_3_5_BRANCHvalgrind-3.5.x
> $ cd valgrind-3.5.x
> $ svn merge -r10859:10860 svn://svn.valgrind.org/valgrind/trunk
> $ svn merge -r10862:10868 svn://svn.valgrind.org/valgrind/trunk
> $ svn merge -r10870:10873 svn://svn.valgrind.org/valgrind/trunk
> $ svn diff
> Property changes on: .
> ___________________________________________________________________
> Deleted: svn:mergeinfo
>
> Index: configure.in
> ===================================================================
> --- configure.in (revision 10874)
> +++ configure.in (working copy)
> @@ -1470,14 +1470,6 @@
>
>
>
> #--------------------------------------------------------------------------
>-- -# Check for /proc filesystem
> -#-------------------------------------------------------------------------
>--- -AC_CHECK_FILES(/proc/self/fd /proc/self/exe /proc/self/maps,
> - [ AC_DEFINE([HAVE_PROC], 1, [can use /proc filesystem]) ],
> - [])
> -
> -
> -#-------------------------------------------------------------------------
>--- # Checks for C header files.
>
> #--------------------------------------------------------------------------
>--
>
> @@ -1534,6 +1526,7 @@
> pthread_rwlock_timedrdlock \
> pthread_rwlock_timedwrlock \
> pthread_spin_lock \
> + readlinkat \
> semtimedop \
> signalfd \
> sigwaitinfo \
>
> Property changes on: none/tests
> ___________________________________________________________________
> Modified: svn:ignore
> - *.dSYM
> *.so
> *.stderr.diff*
> *.stderr.out
> *.stdout.diff*
> *.stdout.out
> .deps
> ansi
> args
> async-sigs
> as_mmap
> as_shm
> bitfield1
> blockfault
> bug129866
> closeall
> coolo_sigaction
> coolo_strlen
> discard
> exec-sigmask
> execve
> faultstatus
> fcntl_setown
> fdleak_cmsg
> fdleak_creat
> fdleak_dup
> fdleak_dup2
> fdleak_fcntl
> fdleak_ipv4
> fdleak_open
> fdleak_pipe
> fdleak_socketpair
> floored
> fork
> fucomip
> gxx304
> insn_basic
> insn_basic.c
> insn_cmov
> insn_cmov.c
> insn_fpu
> insn_fpu.c
> insn_mmx
> insn_mmx.c
> insn_mmxext
> insn_mmxext.c
> insn_sse
> insn_sse.c
> insn_sse2
> insn_sse2.c
> Makefile
> Makefile.in
> manythreads
> map_unaligned
> map_unmap
> mq
> mremap
> mremap2
> munmap_exe
> nestedfns
> pending
> pluto
> pth_atfork1
> pth_blockedsig
> pth_cancel1
> pth_cancel2
> pth_cvsimple
> pth_detached
> pth_empty
> pth_exit
> pth_exit2
> pth_mutexspeed
> pth_once
> pth_rwlock
> pth_semaphore1
> pth_simple_mutex
> pth_simple_threads
> pth_specific
> pth_stackalign
> pth_yield
> rcrl
> readline1
> resolv
> res_search
> rlimit_nofile
> selfrun
> sem
> semlimit
> sha1_test
> shortpush
> shorts
> sigstackgrowth
> smc1
> stackgrowth
> susphello
> syscall-restart1
> syscall-restart2
> syslog
> system
> thread-exits
> threaded-fork
> threadederrno
> timestamp
> tls
> vgcore.*
> vgprintf
> yield
>
> + *.dSYM
> *.so
> *.stderr.diff*
> *.stderr.out
> *.stdout.diff*
> *.stdout.out
> .deps
> ansi
> args
> async-sigs
> as_mmap
> as_shm
> bitfield1
> blockfault
> bug129866
> closeall
> coolo_sigaction
> coolo_strlen
> discard
> exec-sigmask
> execve
> faultstatus
> fcntl_setown
> fdleak_cmsg
> fdleak_creat
> fdleak_dup
> fdleak_dup2
> fdleak_fcntl
> fdleak_ipv4
> fdleak_open
> fdleak_pipe
> fdleak_socketpair
> floored
> fork
> fucomip
> gxx304
> insn_basic
> insn_basic.c
> insn_cmov
> insn_cmov.c
> insn_fpu
> insn_fpu.c
> insn_mmx
> insn_mmx.c
> insn_mmxext
> insn_mmxext.c
> insn_sse
> insn_sse.c
> insn_sse2
> insn_sse2.c
> Makefile
> Makefile.in
> manythreads
> map_unaligned
> map_unmap
> mq
> mremap
> mremap2
> munmap_exe
> nestedfns
> pending
> pluto
> procfs-cmdline-exe
> pth_atfork1
> pth_blockedsig
> pth_cancel1
> pth_cancel2
> pth_cvsimple
> pth_detached
> pth_empty
> pth_exit
> pth_exit2
> pth_mutexspeed
> pth_once
> pth_rwlock
> pth_semaphore1
> pth_simple_mutex
> pth_simple_threads
> pth_specific
> pth_stackalign
> pth_yield
> rcrl
> readline1
> resolv
> res_search
> rlimit_nofile
> selfrun
> sem
> semlimit
> sha1_test
> shortpush
> shorts
> sigstackgrowth
> smc1
> stackgrowth
> susphello
> syscall-restart1
> syscall-restart2
> syslog
> system
> thread-exits
> threaded-fork
> threadederrno
> timestamp
> tls
> vgcore.*
> vgprintf
> yield
>
>
> Index: none/tests/Makefile.am
> ===================================================================
> --- none/tests/Makefile.am (revision 10874)
> +++ none/tests/Makefile.am (working copy)
> @@ -88,6 +88,10 @@
> munmap_exe.stderr.exp munmap_exe.vgtest \
> nestedfns.stderr.exp nestedfns.stdout.exp nestedfns.vgtest \
> pending.stdout.exp pending.stderr.exp pending.vgtest \
> + procfs-linux.stderr.exp-with-readlinkat \
> + procfs-linux.stderr.exp-without-readlinkat \
> + procfs-linux.vgtest \
> + procfs-non-linux.stderr.exp procfs-non-linux.vgtest \
> pth_atfork1.stderr.exp pth_atfork1.stdout.exp pth_atfork1.vgtest \
> pth_blockedsig.stderr.exp \
> pth_blockedsig.stdout.exp pth_blockedsig.vgtest \
> @@ -155,6 +159,7 @@
> munmap_exe map_unaligned map_unmap mq \
> nestedfns \
> pending \
> + procfs-cmdline-exe \
> pth_atfork1 pth_blockedsig pth_cancel1 pth_cancel2 pth_cvsimple \
> pth_empty pth_exit pth_exit2 pth_mutexspeed pth_once pth_rwlock \
> pth_stackalign \
> Index: coregrind/m_syswrap/syswrap-generic.c
> ===================================================================
> --- coregrind/m_syswrap/syswrap-generic.c (revision 10874)
> +++ coregrind/m_syswrap/syswrap-generic.c (working copy)
> @@ -3526,7 +3526,7 @@
> }
> PRE_MEM_RASCIIZ( "open(filename)", ARG1 );
>
> -#if HAVE_PROC
> +#if defined(VGO_linux)
> /* Handle the case where the open is of /proc/self/cmdline or
> /proc/<pid>/cmdline, and just give it a copy of the fd for the
> fake file we cooked up at startup (in m_main). Also, seek the
> @@ -3551,7 +3551,7 @@
> return;
> }
> }
> -#endif // HAVE_PROC
> +#endif // defined(VGO_linux)
>
> /* Otherwise handle normally */
> *flags |= SfMayBlock;
> @@ -3674,7 +3674,7 @@
> PRE_MEM_WRITE( "readlink(buf)", ARG2,ARG3 );
>
> {
> -#if HAVE_PROC
> +#if defined(VGO_linux)
> /*
> * Handle the case where readlink is looking at /proc/self/exe or
> * /proc/<pid>/exe.
> @@ -3690,7 +3690,7 @@
> SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, (UWord)name,
> ARG2, ARG3));
> } else
> -#endif // HAVE_PROC
> +#endif // defined(VGO_linux)
> {
> /* Normal case */
> SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, ARG1, ARG2,
> ARG3));
> Index: coregrind/m_aspacemgr/aspacemgr-linux.c
> ===================================================================
> --- coregrind/m_aspacemgr/aspacemgr-linux.c (revision 10874)
> +++ coregrind/m_aspacemgr/aspacemgr-linux.c (working copy)
> @@ -2986,7 +2986,7 @@
> #endif // HAVE_MREMAP
>
>
> -#if HAVE_PROC
> +#if defined(VGO_linux)
>
> /*-----------------------------------------------------------------*/
> /*--- ---*/
> @@ -3493,7 +3493,7 @@
> return !css_overflowed;
> }
>
> -#endif // HAVE_PROC
> +#endif // defined(VGO_linux)
>
>
> #endif // defined(VGO_linux) || defined(VGO_darwin)
> Index: coregrind/m_main.c
> ===================================================================
> --- coregrind/m_main.c (revision 10874)
> +++ coregrind/m_main.c (working copy)
> @@ -1802,7 +1802,7 @@
> // when it tries to open /proc/<pid>/cmdline for itself.
> // p: setup file descriptors
> //--------------------------------------------------------------
> -#if !HAVE_PROC
> +#if !defined(VGO_linux)
> // client shouldn't be using /proc!
> VG_(cl_cmdline_fd) = -1;
> #else
|
|
From: Bart V. A. <bar...@gm...> - 2009-08-29 06:11:07
|
On Fri, Aug 28, 2009 at 5:19 PM, Julian Seward <js...@ac...> wrote: > Ok for the merge, but please only merge the absolute minimal set > of changes needed to fix the bug on the branch: > > * removal of # Check for /proc filesystem > * changes in coregrind/m_syswrap/syswrap-generic.c, > coregrind/m_aspacemgr/aspacemgr-linux.c, > coregrind/m_main.c > > Don't merge the new regression tests nor the svn:ignore changes. > Obviously it's good that the bug is fixed on the branch. However > I don't want to have to re-verify that there are no regtest build > problems on the branch. Hence the minimal change set only. OK, done. Bart. |
|
From: Julian S. <js...@ac...> - 2009-08-30 20:33:43
|
On Saturday 29 August 2009 08:10:53 am Bart Van Assche wrote: > OK, done. Thanks. J |