|
From: Guy I. <tky...@cc...> - 2004-08-18 16:24:59
|
(I'm not on the list, so please cc me replies)
This is also my first contact with the valgrind list, so bear with me.
It seems that some of the Sys V ipc syscall checkes are
incorrect in coregrind/vg_syscalls.c atleast in version 2.1.2.
msgctl return value checks are faulty (res == 0 indicates success
and that the buf has been touched by the kernel).
semctl lacked manipulation altogether, I added this only partially
but if the patch (below) seems ok, I can also write support
for the rest of the semctl functionality (namely the GETALL is
still missing, which is also quite a bit trickier to implement).
Anyway, they seemed to work now.
cheers,
Tommi Kyntola tky...@cc...
ps. without looking into it, it seems rather nasty that
valgrind dumps core when ran on -gstabs+ compiled binaries
(given that stabs are info for gdb and when valgrind
is being used so is usually gdb)
diff -Naur valgrind-2.1.2-vanilla/coregrind/vg_syscalls.c valgrind-2.1.2-semfix/coregrind/vg_syscalls.c
--- valgrind-2.1.2-vanilla/coregrind/vg_syscalls.c 2004-07-17 12:43:13.000000000 +0300
+++ valgrind-2.1.2-semfix/coregrind/vg_syscalls.c 2004-08-18 19:20:17.768761016 +0300
@@ -2538,6 +2538,19 @@
MAYBE_PRINTF("getuid32 ( )\n");
}
+#if defined(__GNU_LIBRARY__) && !defined(_SEM_SEMUN_UNDEFINED)
+/* union semun is defined by including <sys/sem.h> */
+#else
+/* according to X/OPEN we have to define it ourselves */
+union semun {
+ int val; /* value for SETVAL */
+ struct semid_ds *buf; /* buffer for IPC_STAT, IPC_SET */
+ unsigned short *array; /* array for GETALL, SETALL */
+ /* Linux specific part: */
+ struct seminfo *__buf; /* buffer for IPC_INFO */
+};
+#endif
+
PRE(ipc)
{
MAYBE_PRINTF("ipc ( %d, %d, %d, %d, %p, %d )\n",
@@ -2548,8 +2561,47 @@
arg3 * sizeof(struct sembuf) );
break;
case 2: /* IPCOP_semget */
+ break;
case 3: /* IPCOP_semctl */
+ {
+ switch (arg4 /* cmd */) {
+ case IPC_STAT:
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ SYSCALL_TRACK( pre_mem_write, tid, "semctl(buf)", args->buf,
+ sizeof(struct semid_ds) );
+ }
+ break;
+ case IPC_SET:
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ SYSCALL_TRACK( pre_mem_read, tid, "semctl(buf)", args->buf,
+ sizeof(struct semid_ds) );
+ }
+ break;
+# if defined(IPC_64)
+ case IPC_STAT|IPC_64:
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ SYSCALL_TRACK( pre_mem_write, tid, "semctl(buf)", args->buf,
+ sizeof(struct semid64_ds) );
+ }
+ break;
+# endif
+# if defined(IPC_64)
+ case IPC_SET|IPC_64:
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ SYSCALL_TRACK( pre_mem_read, tid, "semctl(buf)", args->buf,
+ sizeof(struct semid64_ds) );
+ }
+ break;
+# endif
+ default:
+ break;
+ }
break;
+ }
case 4: /* IPCOP_semtimedop */
SYSCALL_TRACK( pre_mem_read, tid, "semtimedop(sops)", arg5,
arg3 * sizeof(struct sembuf) );
@@ -2685,7 +2737,41 @@
switch (arg1 /* call */) {
case 1: /* IPCOP_semop */
case 2: /* IPCOP_semget */
+ break;
case 3: /* IPCOP_semctl */
+ {
+ switch (arg4 /* cmd */) {
+ case IPC_STAT:
+ if ( res == 0 ) {
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ VG_TRACK( post_mem_write, args->buf,
+ sizeof(struct semid_ds) );
+ }
+ }
+ break;
+ case IPC_SET:
+ break;
+# if defined(IPC_64)
+ case IPC_STAT|IPC_64:
+ if ( res == 0 ) {
+ if (arg5 != 0) {
+ union semun * args = (union semun *)arg5;
+ VG_TRACK( post_mem_write, args->buf,
+ sizeof(struct semid64_ds) );
+ }
+ }
+ break;
+# endif
+# if defined(IPC_64)
+ case IPC_SET|IPC_64:
+ break;
+# endif
+ default:
+ break;
+ }
+ break;
+ }
case 4: /* IPCOP_semtimedop */
break;
case 11: /* IPCOP_msgsnd */
@@ -2710,7 +2796,7 @@
{
switch (arg3 /* cmd */) {
case IPC_STAT:
- if ( res > 0 ) {
+ if ( res == 0 ) {
VG_TRACK( post_mem_write, arg5,
sizeof(struct msqid_ds) );
}
@@ -2719,7 +2805,7 @@
break;
# if defined(IPC_64)
case IPC_STAT|IPC_64:
- if ( res > 0 ) {
+ if ( res == 0 ) {
VG_TRACK( post_mem_write, arg5,
sizeof(struct msqid64_ds) );
}
|
|
From: Tom H. <th...@cy...> - 2004-08-18 17:02:29
|
In message <412...@cc...>
Guy Incognito <tky...@cc...> wrote:
> This is also my first contact with the valgrind list, so bear with me.
> It seems that some of the Sys V ipc syscall checkes are
> incorrect in coregrind/vg_syscalls.c atleast in version 2.1.2.
>
> msgctl return value checks are faulty (res == 0 indicates success
> and that the buf has been touched by the kernel).
>
> semctl lacked manipulation altogether, I added this only partially
> but if the patch (below) seems ok, I can also write support
> for the rest of the semctl functionality (namely the GETALL is
> still missing, which is also quite a bit trickier to implement).
I fixed all these at the weekend - checking CVS would have saved
you some work ;-)
> ps. without looking into it, it seems rather nasty that
> valgrind dumps core when ran on -gstabs+ compiled binaries
> (given that stabs are info for gdb and when valgrind
> is being used so is usually gdb)
Use DWARF, it's so much nicer ;-)
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|