|
From: Samofatov, N. <NSa...@br...> - 2004-08-11 05:24:24
|
Hi! Firstly, I wanted to thank Valgrind developers for the excellent tool. It really allows to find problems untraceable with other means. Here you may see how Firebird memory pools were integrated with Valgrind memory checking: http://cvs.sourceforge.net/viewcvs.py/firebird/firebird2/src/common/clas ses/alloc.cpp?rev=3D1.58&view=3Dmarkup http://cvs.sourceforge.net/viewcvs.py/firebird/firebird2/src/common/clas ses/alloc.h?rev=3D1.47&view=3Dmarkup Valgrind-specifc stuff is protected with USE_VALGRIND defines. Valgrind also seems to be pretty solid. The only issues I encountered so far when started such a complex beast as Firebird were: 1) IPCOP_semctl subtype of IPC syscall is not handled correctly by Valgrind. For example, when arg4 (cmd) is equal to IPC_STAT then buffer passed in arg5 is filled with semaphore set data. This is not handled and as the result Valgrind complains about uninitialized buffer. This seems to be not the only problem in this area because on machine with a few opened shared memory areas and some semaphores running valgrind --tool=3Dmemcheck ipcs produces very long bunch of bogus errors. Situation in CVS HEAD looks slightly prettier, but this my patch (against 2.1.2) should still be required to make Firebird happy: ---------------------------------- --- vg_syscalls.c 2004-07-17 05:43:13.000000000 -0400 +++ vg_syscalls.c.new 2004-08-11 00:36:40.000000000 -0400 @@ -2548,7 +2548,25 @@ arg3 * sizeof(struct sembuf) ); break; case 2: /* IPCOP_semget */ + break; case 3: /* IPCOP_semctl */ + switch (arg4 /* cmd */) { +# if defined(IPC_64) + case IPC_STAT | IPC_64: { + struct semid64_ds *id_dsp =3D + (struct semid64_ds *)deref_Addr( tid, + (Addr) (&((union semun *)arg5)->buf), + "semctl(semun->buf)" ); + if (id_dsp) { + SYSCALL_TRACK( pre_mem_write, tid, "semctl(buf)", (UInt) id_dsp, + sizeof(struct semid_ds) ); + } + break; + } +# endif + default: + break; + } break; case 4: /* IPCOP_semtimedop */ SYSCALL_TRACK( pre_mem_read, tid, "semtimedop(sops)", arg5, @@ -2685,7 +2703,28 @@ switch (arg1 /* call */) { case 1: /* IPCOP_semop */ case 2: /* IPCOP_semget */ + break; case 3: /* IPCOP_semctl */ + { + switch (arg4 /* cmd */) { +# if defined(IPC_64) + case IPC_STAT | IPC_64: { + struct semid64_ds* id_dsp =3D + (struct semid64_ds *)deref_Addr( tid, + (Addr) (&((union semun *)arg5)->buf), + "semctl(semun->buf)" ); + if (id_dsp) { + VG_TRACK( post_mem_write, (UInt) id_dsp, + sizeof(struct semid64_ds) ); + } + break; + } +# endif + default: + break; + } + break; + } case 4: /* IPCOP_semtimedop */ break; case 11: /* IPCOP_msgsnd */ ---------------------------------- Interestingly enough, I had to fix bugs in this syscall code inside Linux kernel when tested 32-bit Firebird build on UltraSparc Linux. Maybe semaphores and shared memory API is just not popular enough to be stable? :-) 2) When known zero length and uninitialized pointers are passed to Valgrind memcpy() replacement routine it complains about bad pointers passed. I don't know if Firebird behavior is perfect from standard point of view, but this code works on many platforms and with many compilers. So I'm not sure if this problem should be fixed in Firebird or in Valgrind. Nickolay Samofatov |
|
From: Tom H. <th...@cy...> - 2004-08-11 06:26:25
|
In message <661...@md...>
"Samofatov, Nickolay" <NSa...@br...> wrote:
> 1) IPCOP_semctl subtype of IPC syscall is not handled correctly by
> Valgrind. For example, when arg4 (cmd) is equal to IPC_STAT then buffer
> passed in arg5 is filled with semaphore set data.
> This is not handled and as the result Valgrind complains about
> uninitialized buffer.
> This seems to be not the only problem in this area because on machine
> with a few opened shared memory areas and some semaphores running
> valgrind --tool=memcheck ipcs
> produces very long bunch of bogus errors.
Can you raise a bug for this please so that your patch doesn't
get lost.
> 2) When known zero length and uninitialized pointers are passed to
> Valgrind memcpy() replacement routine it complains about bad pointers
> passed.
> I don't know if Firebird behavior is perfect from standard point of
> view, but this code works on many platforms and with many compilers.
> So I'm not sure if this problem should be fixed in Firebird or in
> Valgrind.
It's probably worth raising a bug for this as well so we can
consider it properly.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|