Fix false positive in sys_clone on amd64 when optional args are not given (e.g. child...

rev 10493 fixed bug 117564 in syswrap-x86-linux.c.
This commit fixes the same problem in syswrap-amd64-linux.c.
The problem makes memcheck/tests/linux/stack_switch fails (at least on gcc20)
with unexpected
   ==802== Syscall param clone(child_tidptr) contains uninitialised byte(s)
The problem originates from always checking 3 optional args PRE_read,
while these should be checked only if the corresponding flags are set.

syswrap-{arm,ppc32,ppc64}-linux.c seems to have the same problem
(but no visible effect) : VKI_CLONE_PARENT_SETTID,VKI_CLONE_CHILD_SETTID
and VKI_CLONE_SETTLS not properly handled in the PRE part.

syswrap-s390x-linux.c seems to have the VKI_CLONE_SETTLS part wrong,
but VKI_CLONE_PARENT_SETTID and VKI_CLONE_CHILD_SETTID correct.

Commiting a fix just for amd64 for now.
We probably better make some common code in syswrap-generic.c
to regroup all similar platforms.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12586

diff --git a/NEWS b/NEWS
index 4349c4ac221324ab89ea90402f2efdc8a50b0012..18a32cab6f0088328c57139695b2c42a655a8e58 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -106,6 +106,7 @@ n-i-bz  s390x: Shadow registers can now be examined using vgdb
 299756  For symmetry, --free-fill must be ignored for MEMPOOL_FREE and FREELIKE client requests
 n-i-bz  Bypass gcc4.4/4.5 wrong code generation causing out of memory or asserts
 n-i-bz  Add missing gdbserver xml files for shadow registers for ppc32
+n-i-bz  Fix false positive in sys_clone on amd64 when optional args are not given (e.g. child_tidptr)
 
 Release 3.7.0 (5 November 2011)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/coregrind/m_syswrap/syswrap-amd64-linux.c b/coregrind/m_syswrap/syswrap-amd64-linux.c
index 240d3267f7b734db46c8542c74dfb2b2b1e1a38e..31b106160cf527f799d7ba907ebd83366c8ca9c7 100644 (file)
--- a/coregrind/m_syswrap/syswrap-amd64-linux.c
+++ b/coregrind/m_syswrap/syswrap-amd64-linux.c
@@ -399,21 +399,35 @@ PRE(sys_clone)
    ULong cloneflags;
 
    PRINT("sys_clone ( %lx, %#lx, %#lx, %#lx, %#lx )",ARG1,ARG2,ARG3,ARG4,ARG5);
-   PRE_REG_READ5(int, "clone",
+   PRE_REG_READ2(int, "clone",
                  unsigned long, flags,
-                 void *, child_stack,
-                 int *, parent_tidptr,
-                 int *, child_tidptr,
-                 void *, tlsaddr);
+                 void *, child_stack);
 
    if (ARG1 & VKI_CLONE_PARENT_SETTID) {
+      if (VG_(tdict).track_pre_reg_read) {
+         PRA3("clone", int *, parent_tidptr);
+      }
       PRE_MEM_WRITE("clone(parent_tidptr)", ARG3, sizeof(Int));
       if (!VG_(am_is_valid_for_client)(ARG3, sizeof(Int), VKI_PROT_WRITE)) {
          SET_STATUS_Failure( VKI_EFAULT );
          return;
       }
    }
+   if (ARG1 & VKI_CLONE_SETTLS) {
+      if (VG_(tdict).track_pre_reg_read) {
+         PRA4("clone", vki_modify_ldt_t *, tlsinfo);
+      }
+      PRE_MEM_READ("clone(tlsinfo)", ARG4, sizeof(vki_modify_ldt_t));
+      if (!VG_(am_is_valid_for_client)(ARG4, sizeof(vki_modify_ldt_t), 
+                                             VKI_PROT_READ)) {
+         SET_STATUS_Failure( VKI_EFAULT );
+         return;
+      }
+   }
    if (ARG1 & (VKI_CLONE_CHILD_SETTID | VKI_CLONE_CHILD_CLEARTID)) {
+      if (VG_(tdict).track_pre_reg_read) {
+         PRA5("clone", int *, child_tidptr);
+      }
       PRE_MEM_WRITE("clone(child_tidptr)", ARG4, sizeof(Int));
       if (!VG_(am_is_valid_for_client)(ARG4, sizeof(Int), VKI_PROT_WRITE)) {
          SET_STATUS_Failure( VKI_EFAULT );