[Valgrind-developers] valgrind: r12586: Fix false positive in sys_clone on amd64 when optional args are not given (e.g. child_tidptr)

[<sv...@va...>]

philippe	2012-05-27 00:08:41 +0100 (Sun, 27 May 2012)

  New Revision: 12586

  Log:
    Fix false positive in sys_clone on amd64 when optional args are not given (e.g. child_tidptr)
    
    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.

  Modified files:
    trunk/NEWS
    trunk/coregrind/m_syswrap/syswrap-amd64-linux.c


  Modified: trunk/NEWS (+1 -0)
===================================================================
--- trunk/NEWS	2012-05-26 00:22:39 -23:00 (rev 12585)
+++ trunk/NEWS	2012-05-27 00:08:41 -23:00 (rev 12586)
@@ -106,6 +106,7 @@
 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)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  Modified: trunk/coregrind/m_syswrap/syswrap-amd64-linux.c (+19 -5)
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-amd64-linux.c	2012-05-26 00:22:39 -23:00 (rev 12585)
+++ trunk/coregrind/m_syswrap/syswrap-amd64-linux.c	2012-05-27 00:08:41 -23:00 (rev 12586)
@@ -399,21 +399,35 @@
    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 );