|
From: Petar J. <mip...@gm...> - 2012-10-24 16:22:50
|
Hey Philippe,
how about adding a patch like this one on the top of it?
Regards,
Petar
Index: coregrind/m_syswrap/syswrap-linux.c
===================================================================
--- coregrind/m_syswrap/syswrap-linux.c (revision 13082)
+++ coregrind/m_syswrap/syswrap-linux.c (working copy)
@@ -3322,20 +3322,24 @@
{
PRINT("sys_ipc ( %ld, %ld, %ld, %ld, %#lx, %ld )",
ARG1,ARG2,ARG3,ARG4,ARG5,ARG6);
- // XXX: this is simplistic -- some args are not used in all circumstances.
- PRE_REG_READ6(int, "ipc",
- vki_uint, call, int, first, int, second, int, third,
- void *, ptr, long, fifth);
switch (ARG1 /* call */) {
case VKI_SEMOP:
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
ML_(generic_PRE_sys_semop)( tid, ARG2, ARG5, ARG3 );
*flags |= SfMayBlock;
break;
case VKI_SEMGET:
+ PRE_REG_READ4(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third);
break;
case VKI_SEMCTL:
{
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
UWord arg;
if (semctl_cmd_has_4args(ARG4))
arg = deref_Addr( tid, ARG5, "semctl(arg)" );
@@ -3345,25 +3349,33 @@
break;
}
case VKI_SEMTIMEDOP:
+ PRE_REG_READ6(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr, long, fifth);
ML_(generic_PRE_sys_semtimedop)( tid, ARG2, ARG5, ARG3, ARG6 );
*flags |= SfMayBlock;
break;
case VKI_MSGSND:
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
ML_(linux_PRE_sys_msgsnd)( tid, ARG2, ARG5, ARG3, ARG4 );
if ((ARG4 & VKI_IPC_NOWAIT) == 0)
*flags |= SfMayBlock;
break;
case VKI_MSGRCV:
{
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
Addr msgp;
Word msgtyp;
- msgp = deref_Addr( tid,
- (Addr) (&((struct vki_ipc_kludge *)ARG5)->msgp),
- "msgrcv(msgp)" );
- msgtyp = deref_Addr( tid,
- (Addr) (&((struct vki_ipc_kludge *)ARG5)->msgtyp),
- "msgrcv(msgp)" );
+ msgp = deref_Addr( tid, (Addr) (&((struct vki_ipc_kludge *)ARG5)->msgp),
+ "msgrcv(msgp)" );
+ msgtyp = deref_Addr( tid,
+ (Addr) (&((struct vki_ipc_kludge *)ARG5)->msgtyp),
+ "msgrcv(msgp)" );
ML_(linux_PRE_sys_msgrcv)( tid, ARG2, msgp, ARG3, msgtyp, ARG4 );
@@ -3372,12 +3384,19 @@
break;
}
case VKI_MSGGET:
+ PRE_REG_READ3(int, "ipc", vki_uint, call, int, first, int, second);
break;
case VKI_MSGCTL:
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
ML_(linux_PRE_sys_msgctl)( tid, ARG2, ARG3, ARG5 );
break;
case VKI_SHMAT:
{
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
UWord w;
PRE_MEM_WRITE( "shmat(raddr)", ARG4, sizeof(Addr) );
w = ML_(generic_PRE_sys_shmat)( tid, ARG2, ARG5, ARG3 );
@@ -3388,19 +3407,27 @@
break;
}
case VKI_SHMDT:
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
if (!ML_(generic_PRE_sys_shmdt)(tid, ARG5))
SET_STATUS_Failure( VKI_EINVAL );
break;
case VKI_SHMGET:
+ PRE_REG_READ4(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third);
break;
case VKI_SHMCTL: /* IPCOP_shmctl */
+ PRE_REG_READ5(int, "ipc",
+ vki_uint, call, int, first, int, second, int, third,
+ void *, ptr);
ML_(generic_PRE_sys_shmctl)( tid, ARG2, ARG3, ARG5 );
break;
default:
VG_(message)(Vg_DebugMsg, "FATAL: unhandled syscall(ipc) %ld\n", ARG1 );
VG_(core_panic)("... bye!\n");
break; /*NOTREACHED*/
- }
+ }
}
POST(sys_ipc)
On Tue, Oct 23, 2012 at 11:38 PM, <sv...@va...> wrote:
> philippe 2012-10-23 22:38:52 +0100 (Tue, 23 Oct 2012)
>
> New Revision: 13082
>
> Log:
> fix 123837 semctl system call: 4rth argument is optional, depending on cmd
>
> Depending on the semctl command (arg3), arg4 might or might not be needed.
> The PRE(sys_ipc) multiplexed syscall for semctl was always checking
> all 4 args.
>
> The fix consists in dereferencing the 4th arg (which in sys_ipc is ARG5)
> only if the semctl syscall cmd implies 4 arguments.
> This avoids the false positive on linux x86.
>
> Note that PRE(sys_ipc) is still too simplistic as it assumes
> that 6 args are always read, which is not the case.
> This seems to cause false positive on mips:
> memcheck on none/tests/sem gives:
> Syscall param ipc(fifth) contains uninitialised byte(s)
>
> It would be nice to implement the multiplexed PRE(sys_ipc) by
> calling the PRE(sys_xxxx) similar PRE, depending on ARG1 of sys_ipc.
> This would then avoid the simplistic PRE(sys_ipc) logic without duplicating
> the logic in PRE(sys_semctl) (and all other sys_ipc multiplexed syscalls).
> However, I found no easy way to do that.
>
> With the current fix, some logic about semctl is partially duplicated between
> the PRE(sys_ipc) (for platforms such as x86 having a multiplexed sys call)
> and PRE(sys_semctl) (for platforms such as amd64, having a direct sys call)
> to fix the false positive encountered on x86.
>
> Modified files:
> trunk/NEWS
> trunk/coregrind/m_syswrap/syswrap-linux.c
>
>
> Modified: trunk/coregrind/m_syswrap/syswrap-linux.c (+27 -2)
> ===================================================================
> --- trunk/coregrind/m_syswrap/syswrap-linux.c 2012-10-23 19:03:28 +01:00 (rev 13081)
> +++ trunk/coregrind/m_syswrap/syswrap-linux.c 2012-10-23 22:38:52 +01:00 (rev 13082)
> @@ -3301,6 +3301,23 @@
> return *a_p;
> }
>
> +Bool semctl_cmd_has_4args (UWord cmd)
> +{
> + switch (cmd & ~VKI_IPC_64)
> + {
> + case VKI_IPC_INFO:
> + case VKI_SEM_INFO:
> + case VKI_IPC_STAT:
> + case VKI_SEM_STAT:
> + case VKI_IPC_SET:
> + case VKI_GETALL:
> + case VKI_SETALL:
> + return True;
> + default:
> + return False;
> + }
> +}
> +
> PRE(sys_ipc)
> {
> PRINT("sys_ipc ( %ld, %ld, %ld, %ld, %#lx, %ld )",
> @@ -3319,7 +3336,11 @@
> break;
> case VKI_SEMCTL:
> {
> - UWord arg = deref_Addr( tid, ARG5, "semctl(arg)" );
> + UWord arg;
> + if (semctl_cmd_has_4args(ARG4))
> + arg = deref_Addr( tid, ARG5, "semctl(arg)" );
> + else
> + arg = 0;
> ML_(generic_PRE_sys_semctl)( tid, ARG2, ARG3, ARG4, arg );
> break;
> }
> @@ -3391,7 +3412,11 @@
> break;
> case VKI_SEMCTL:
> {
> - UWord arg = deref_Addr( tid, ARG5, "semctl(arg)" );
> + UWord arg;
> + if (semctl_cmd_has_4args(ARG4))
> + arg = deref_Addr( tid, ARG5, "semctl(arg)" );
> + else
> + arg = 0;
> ML_(generic_POST_sys_semctl)( tid, RES, ARG2, ARG3, ARG4, arg );
> break;
> }
>
> Modified: trunk/NEWS (+1 -0)
> ===================================================================
> --- trunk/NEWS 2012-10-23 19:03:28 +01:00 (rev 13081)
> +++ trunk/NEWS 2012-10-23 22:38:52 +01:00 (rev 13082)
> @@ -29,6 +29,7 @@
> [381] = fixed in trunk and in 3_8_BRANCH, for 3.8.1
> [382] = fixed in trunk and needs to be made available for 3.8.2 too
>
> +123837 [390] semctl system call: 4rth argument is optional, depending on cmd
> 252955 [390] Impossible to compile with ccache
> 274695 [390] s390x: Support "compare to/from logical" instructions (z196)
> 275800 [390] s390x: Add support for the ecag instruction (part 1)
>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_sfd2d_oct
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|