|
From: Filipe C. <fi...@gm...> - 2009-04-16 22:33:47
|
Hi,
I don't know much about the darwin kernel (yet ;-) ) so I started with a
simpler test... In your notes, the test for post-syscall has something
about an undefined value error in alarm()...
I made a small program that uses alarm (setup + infinite loop + exit()
in the handler) and verified that it is indeed in libSystem:
(I'm running with origin tracking enabled)
--------------
==33057== Conditional jump or move depends on uninitialised value(s)
==33057== at 0x264EB2: alarm (in /usr/lib/libSystem.B.dylib)
==33057== by 0x1FEA: main (in ./a)
==33057== Uninitialised value was created by a stack allocation
==33057== at 0x264E6C: alarm (in /usr/lib/libSystem.B.dylib)
----------------------
So I suppose we could add something like (I'm no suppression genius):
{
macos-Cond-alarm
Memcheck:Cond
fun:alarm
}
But this seems rather small... I don't know if there's a way to force
this alarm() function to be in /usr/lib/libSystem* or if adding an
"obj:" will make valgrind expect two calls inside libSystem.
That's the first one... I'll see another when I have time (and the
lsframe* tests are really weird...)
Regards,
F
|
|
From: Nicholas N. <n.n...@gm...> - 2009-04-16 22:44:45
|
On Fri, Apr 17, 2009 at 8:33 AM, Filipe Cabecinhas <fi...@gm...> wrote:
>
> I don't know much about the darwin kernel (yet ;-) ) so I started with a
> simpler test... In your notes, the test for post-syscall has something
> about an undefined value error in alarm()...
>
> I made a small program that uses alarm (setup + infinite loop + exit()
> in the handler) and verified that it is indeed in libSystem:
> (I'm running with origin tracking enabled)
> --------------
> ==33057== Conditional jump or move depends on uninitialised value(s)
> ==33057== at 0x264EB2: alarm (in /usr/lib/libSystem.B.dylib)
> ==33057== by 0x1FEA: main (in ./a)
> ==33057== Uninitialised value was created by a stack allocation
> ==33057== at 0x264E6C: alarm (in /usr/lib/libSystem.B.dylib)
> ----------------------
>
> So I suppose we could add something like (I'm no suppression genius):
> {
> macos-Cond-alarm
> Memcheck:Cond
> fun:alarm
> }
>
> But this seems rather small... I don't know if there's a way to force
> this alarm() function to be in /usr/lib/libSystem* or if adding an
> "obj:" will make valgrind expect two calls inside libSystem.
It would be hard to do a less general suppression. Before we do that,
I'd rather know why we're getting the error, whether it's genuine or a
false positive, etc.
Nick
|
|
From: Filipe C. <fi...@gm...> - 2009-04-17 12:52:28
Attachments:
valgrind-sem_init_destroy.patch
|
Hi, Nicholas Nethercote wrote: > > It would be hard to do a less general suppression. Before we do that, > I'd rather know why we're getting the error, whether it's genuine or a > false positive, etc. > > Nick I submitted a bug report to Apple... But I'm not expecting them to fix it. How could you see if the error is genuine or a false positive without access to the source? By disassembling and looking at the output? :-) I implemented two brand new Mac OS X system calls... I couldn't believe they had implemented sem_init and sem_destroy when I first saw it! It must have been in Leopard, for POSIX compliance... (But we still have no manpages for the functions). Patch: http://web.ist.utl.pt/~filipe.cabecinhas/patches/valgrind-sem_init_destroy.patch The semlimit test now runs successefully. But I had to test it manually... make regtest blows up: ------------------------------------------------------ make bigcode bz2 fbench ffbench heap sarp tinycc gcc -DHAVE_CONFIG_H -I. -I.. -I.. -I../include -I../include -Winline -Wall -Wshadow -g -O -arch i386 -Wno-long-long -Wno-pointer-sign -Wdeclaration-after-statement -fno-stack-protector -MT bigcode.o -MD -MP -MF .deps/bigcode.Tpo -c -o bigcode.o bigcode.c In file included from bigcode.c:13: ../tests/sys_mman.h: In function 'get_unmapped_page': ../tests/sys_mman.h:25: error: 'MAP_ANONYMOUS' undeclared (first use in this function) ../tests/sys_mman.h:25: error: (Each undeclared identifier is reported only once ../tests/sys_mman.h:25: error: for each function it appears in.) bigcode.c: In function 'main': bigcode.c:40: error: 'MAP_ANONYMOUS' undeclared (first use in this function) make[3]: *** [bigcode.o] Error 1 ------------------------------------------------------ Also, should I try to create a simple test for csops/F_ADDSIGS, when I have the time? Regards, F |
|
From: Nicholas N. <n.n...@gm...> - 2009-04-23 04:41:31
|
On Fri, Apr 17, 2009 at 10:52 PM, Filipe Cabecinhas <fi...@gm...> wrote: > > I implemented two brand new Mac OS X system calls... I couldn't believe they > had implemented sem_init and sem_destroy when I first saw it! It must have > been in Leopard, for POSIX compliance... (But we still have no manpages for > the functions). > > Patch: > http://web.ist.utl.pt/~filipe.cabecinhas/patches/valgrind-sem_init_destroy.patch > > The semlimit test now runs successefully. Committed (r9584), thanks. Nick |
|
From: Nicholas N. <n.n...@gm...> - 2009-04-17 21:50:03
|
On Fri, Apr 17, 2009 at 10:52 PM, Filipe Cabecinhas <fi...@gm...> wrote: >> >> It would be hard to do a less general suppression. Before we do that, >> I'd rather know why we're getting the error, whether it's genuine or a >> false positive, etc. > > I submitted a bug report to Apple... But I'm not expecting them to fix it. > How could you see if the error is genuine or a false positive without access > to the source? By disassembling and looking at the output? :-) Both the kernel (xnu) and the Libc are open source: http://www.opensource.apple.com/darwinsource/10.5.6/ > I implemented two brand new Mac OS X system calls... I couldn't believe they > had implemented sem_init and sem_destroy when I first saw it! It must have > been in Leopard, for POSIX compliance... (But we still have no manpages for > the functions). > > Patch: > http://web.ist.utl.pt/~filipe.cabecinhas/patches/valgrind-sem_init_destroy.patch > > The semlimit test now runs successefully. Great, thanks! I'll commit it next week. > But I had to test it manually... > make regtest blows up: > > ------------------------------------------------------ > make bigcode bz2 fbench ffbench heap sarp tinycc > gcc -DHAVE_CONFIG_H -I. -I.. -I.. -I../include -I../include -Winline -Wall > -Wshadow -g -O -arch i386 -Wno-long-long -Wno-pointer-sign > -Wdeclaration-after-statement -fno-stack-protector -MT bigcode.o -MD -MP -MF > .deps/bigcode.Tpo -c -o bigcode.o bigcode.c > In file included from bigcode.c:13: > ../tests/sys_mman.h: In function 'get_unmapped_page': > ../tests/sys_mman.h:25: error: 'MAP_ANONYMOUS' undeclared (first use in this > function) > ../tests/sys_mman.h:25: error: (Each undeclared identifier is reported only > once > ../tests/sys_mman.h:25: error: for each function it appears in.) > bigcode.c: In function 'main': > bigcode.c:40: error: 'MAP_ANONYMOUS' undeclared (first use in this function) > make[3]: *** [bigcode.o] Error 1 > ------------------------------------------------------ Are you up-to-date? If so, have you rerun autogen.sh lately? > Also, should I try to create a simple test for csops/F_ADDSIGS, when I have > the time? Sure, it can't hurt. I thought about adding your previous test program but it needed root privileges to run, right? There's also memcheck/tests/darwin/scalar.c, which is intended to test all the arguments to all syscalls for undefinedness (at least, most of them). If you can understand how it works, additions to it for the new syscalls (or any other syscalls) would be welcome. Nick |
|
From: Greg P. <gp...@ap...> - 2009-04-17 23:14:16
|
On Apr 17, 2009, at 2:49 PM, Nicholas Nethercote wrote: > Both the kernel (xnu) and the Libc are open source: > http://www.opensource.apple.com/darwinsource/10.5.6/ Note that some parts of the kernel and Libc are not open-source, but in general everything you need for Valgrind is there. Note that most of Libc's syscall functions are trivial wrappers around the kernel syscall. The wrappers are generated by macros and are hard to find. The best bet is to disassemble the Libc function first; if it does nothing except syscall/sysenter and cerror (which sets errno), then you should go straight to xnu's source. >> ../tests/sys_mman.h:25: error: 'MAP_ANONYMOUS' undeclared (first >> use in this >> function) That's spelled MAP_ANON on Mac OS X. (Also, make sure you set either MAP_PRIVATE or MAP_SHARED; if you set neither or both, mmap() may return an error.) -- Greg Parker gp...@ap... Runtime Wrangler |
|
From: Nicholas N. <n.n...@gm...> - 2009-04-18 00:35:36
|
On Sat, Apr 18, 2009 at 8:15 AM, Greg Parker <gp...@ap...> wrote: > >>> ../tests/sys_mman.h:25: error: 'MAP_ANONYMOUS' undeclared (first use in >>> this >>> function) > > That's spelled MAP_ANON on Mac OS X. (Also, make sure you set either > MAP_PRIVATE or MAP_SHARED; if you set neither or both, mmap() may return an > error.) Earlier in the file is a conditional #define that defines MAP_ANONYMOUS as equal to MAP_ANON on Darwin, so the tests can be uniform. I think the problem was that Filipe hadn't rerun autogen.sh recently. Nick |
|
From: Filipe C. <fi...@gm...> - 2009-04-21 22:34:09
Attachments:
test-alarm.c
|
Nicholas Nethercote wrote: > On Fri, Apr 17, 2009 at 10:52 PM, Filipe Cabecinhas <fi...@gm...> wrote: >>> It would be hard to do a less general suppression. Before we do that, >>> I'd rather know why we're getting the error, whether it's genuine or a >>> false positive, etc. >> I submitted a bug report to Apple... But I'm not expecting them to fix it. >> How could you see if the error is genuine or a false positive without access >> to the source? By disassembling and looking at the output? :-) > > Both the kernel (xnu) and the Libc are open source: > http://www.opensource.apple.com/darwinsource/10.5.6/ Heh... I had forgotten the Libc... This is a weird one. I copied the definition of alarm() from libc but the error cause eludes me. Attached is a small test case for the error. The output is: $ valgrind -q --track-origins=yes ./test-alarmallegedly uninitialized value: 0xbffff6e8, start of oitv: 0xbffff6dc, end: 0xbffff6ec ==84680== Conditional jump or move depends on uninitialised value(s) ==84680== at 0x1F38: alrm (test-alarm.c:24) ==84680== by 0x1FAC: main (test-alarm.c:39) ==84680== Uninitialised value was created by a stack allocation ==84680== at 0x1EAE: alrm (test-alarm.c:11) in handler: 14 What's weird is that line 24 reads something that is inside the struct itimerval oitv and setitimer writes to the address specified for the third parameter for sizeof(struct itimerval) bytes, as we can see in syswrap-generic.c: PRE(sys_setitimer) { ... if (ARG3 != (Addr)NULL) PRE_MEM_WRITE( "setitimer(ovalue)", ARG3, sizeof(struct vki_itimerval)); } POST(sys_setitimer) { if (ARG3 != (Addr)NULL) POST_MEM_WRITE(ARG3, sizeof(struct vki_itimerval)); ... vki_itimerval is #defined to itimerval. What could be the error? >> But I had to test it manually... >> make regtest blows up: >> >> ------------------------------------------------------ >> make bigcode bz2 fbench ffbench heap sarp tinycc >> gcc -DHAVE_CONFIG_H -I. -I.. -I.. -I../include -I../include -Winline -Wall >> -Wshadow -g -O -arch i386 -Wno-long-long -Wno-pointer-sign >> -Wdeclaration-after-statement -fno-stack-protector -MT bigcode.o -MD -MP -MF >> .deps/bigcode.Tpo -c -o bigcode.o bigcode.c >> In file included from bigcode.c:13: >> ../tests/sys_mman.h: In function 'get_unmapped_page': >> ../tests/sys_mman.h:25: error: 'MAP_ANONYMOUS' undeclared (first use in this >> function) >> ../tests/sys_mman.h:25: error: (Each undeclared identifier is reported only >> once >> ../tests/sys_mman.h:25: error: for each function it appears in.) >> bigcode.c: In function 'main': >> bigcode.c:40: error: 'MAP_ANONYMOUS' undeclared (first use in this function) >> make[3]: *** [bigcode.o] Error 1 >> ------------------------------------------------------ > > Are you up-to-date? If so, have you rerun autogen.sh lately? Nice. Now it woks. Thanks. >> Also, should I try to create a simple test for csops/F_ADDSIGS, when I have >> the time? > > Sure, it can't hurt. I thought about adding your previous test > program but it needed root privileges to run, right? Ah, yes. That system call wants superuser priveleges for almost anything... So maybe it's not worth it (unless for checking the STATUS command) > There's also memcheck/tests/darwin/scalar.c, which is intended to test > all the arguments to all syscalls for undefinedness (at least, most of > them). If you can understand how it works, additions to it for the > new syscalls (or any other syscalls) would be welcome. I'll take a peek at it, when I can. :-) Regards, Filipe |
|
From: Greg P. <gp...@ap...> - 2009-04-21 23:05:49
|
On Apr 21, 2009, at 3:33 PM, Filipe Cabecinhas wrote:
> This is a weird one. I copied the definition of alarm() from libc
> but the error cause eludes me. Attached is a small test case for the
> error.
> The output is:
> $ valgrind -q --track-origins=yes ./test-alarmallegedly
> uninitialized value: 0xbffff6e8, start of oitv: 0xbffff6dc, end:
> 0xbffff6ec
> ==84680== Conditional jump or move depends on uninitialised value(s)
> ==84680== at 0x1F38: alrm (test-alarm.c:24)
> ==84680== by 0x1FAC: main (test-alarm.c:39)
> ==84680== Uninitialised value was created by a stack allocation
> ==84680== at 0x1EAE: alrm (test-alarm.c:11)
> in handler: 14
>
> What's weird is that line 24 reads something that is inside the
> struct itimerval oitv and setitimer writes to the address specified
> for the third parameter for sizeof(struct itimerval) bytes, as we
> can see in syswrap-generic.c:
>
> PRE(sys_setitimer)
> {
> ...
> if (ARG3 != (Addr)NULL)
> PRE_MEM_WRITE( "setitimer(ovalue)", ARG3, sizeof(struct
> vki_itimerval));
> }
>
> POST(sys_setitimer)
> {
> if (ARG3 != (Addr)NULL)
> POST_MEM_WRITE(ARG3, sizeof(struct vki_itimerval));
> ...
>
> vki_itimerval is #defined to itimerval.
>
> What could be the error?
The error is the syscall_table entry in syswrap-darwin.c, at least if
yours looks like mine:
GENX_(__NR_setitimer, sys_setitimer),
That should be GENXY; as written it fails to run the POST handler.
Aside: struct itimerval is two struct timevals, and 64-bit struct
timeval has an alignment pad in the middle, so you should use
PRE_timeval_READ/PRE_timeval_WRITE/POST_timeval_WRITE. Using
PRE_MEM_READ(sizeof(struct itimerval)) to check setitimer(ARG2) will
cause false error reports.
--
Greg Parker gp...@ap... Runtime Wrangler
|
|
From: Nicholas N. <n.n...@gm...> - 2009-04-21 23:28:15
|
On Wed, Apr 22, 2009 at 9:05 AM, Greg Parker <gp...@ap...> wrote: > > The error is the syscall_table entry in syswrap-darwin.c, at least if yours > looks like mine: > > GENX_(__NR_setitimer, sys_setitimer), > > That should be GENXY; as written it fails to run the POST handler. > > Aside: struct itimerval is two struct timevals, and 64-bit struct timeval > has an alignment pad in the middle, so you should use > PRE_timeval_READ/PRE_timeval_WRITE/POST_timeval_WRITE. Using > PRE_MEM_READ(sizeof(struct itimerval)) to check setitimer(ARG2) will cause > false error reports. Filipe, if you could create a patch that fixes these problems that'd be very helpful! Thanks. Nick |
|
From: Filipe C. <fi...@gm...> - 2009-04-22 09:22:27
Attachments:
valgrind-get-setitimer.patch
|
Hi Nicholas Nethercote wrote: > On Wed, Apr 22, 2009 at 9:05 AM, Greg Parker <gp...@ap...> wrote: >> The error is the syscall_table entry in syswrap-darwin.c, at least if yours >> looks like mine: >> >> GENX_(__NR_setitimer, sys_setitimer), >> >> That should be GENXY; as written it fails to run the POST handler. >> >> Aside: struct itimerval is two struct timevals, and 64-bit struct timeval >> has an alignment pad in the middle, so you should use >> PRE_timeval_READ/PRE_timeval_WRITE/POST_timeval_WRITE. Using >> PRE_MEM_READ(sizeof(struct itimerval)) to check setitimer(ARG2) will cause >> false error reports. > > Filipe, if you could create a patch that fixes these problems that'd > be very helpful! Thanks. > > Nick It's done. I also included Greg's suggestions and changed getitimer and setitimer's PRE_MEM_READ(..., sizeof(struct vki_itimerval) to two PRE_timeval_READ(...) as well as the writes. The same version of the patch is online at the usual place, as always (I don't know if sourceforge keeps the attachments so I keep them in my site too. Regards, Filipe |
|
From: Nicholas N. <n.n...@gm...> - 2009-04-23 04:50:29
|
On Wed, Apr 22, 2009 at 7:22 PM, Filipe Cabecinhas <fi...@gm...> wrote: > > It's done. I also included Greg's suggestions and changed getitimer and > setitimer's PRE_MEM_READ(..., sizeof(struct vki_itimerval) to two > PRE_timeval_READ(...) as well as the writes. Committed with minor changes as r9585 -- you used PRE_timeval_WRITE in the POST handlers; I changed them to POST_timeval_WRITE. Thanks. |
|
From: Filipe C. <fi...@gm...> - 2009-04-23 08:15:21
|
Heh, my bad. Thanks, Filipe Nicholas Nethercote wrote: > On Wed, Apr 22, 2009 at 7:22 PM, Filipe Cabecinhas <fi...@gm...> wrote: >> It's done. I also included Greg's suggestions and changed getitimer and >> setitimer's PRE_MEM_READ(..., sizeof(struct vki_itimerval) to two >> PRE_timeval_READ(...) as well as the writes. > > Committed with minor changes as r9585 -- you used PRE_timeval_WRITE in > the POST handlers; I changed them to POST_timeval_WRITE. Thanks. |
|
From: Filipe C. <fi...@gm...> - 2009-04-28 19:19:49
Attachments:
valgrind-scalar-csops-sem-test.patch
|
Hi,
Filipe Cabecinhas wrote:
>> There's also memcheck/tests/darwin/scalar.c, which is intended to test
>> all the arguments to all syscalls for undefinedness (at least, most of
>> them). If you can understand how it works, additions to it for the
>> new syscalls (or any other syscalls) would be welcome.
> I'll take a peek at it, when I can. :-)
I took a peek at it and inserted some calls to csops and sem_{init,destroy}.
Here is the patch.
I think I understood what the Xs Ym mean...
Xs: X "scalar" arguments
Ym: Y "memory" (pointer) arguments.
Regards,
Filipe
|
|
From: Nicholas N. <n.n...@gm...> - 2009-05-07 01:00:27
|
On Wed, Apr 29, 2009 at 5:19 AM, Filipe Cabecinhas <fi...@gm...> wrote:
>>> There's also memcheck/tests/darwin/scalar.c, which is intended to test
>>> all the arguments to all syscalls for undefinedness (at least, most of
>>> them). If you can understand how it works, additions to it for the
>>> new syscalls (or any other syscalls) would be welcome.
>>
>> I'll take a peek at it, when I can. :-)
>
> I took a peek at it and inserted some calls to csops and sem_{init,destroy}.
>
> Here is the patch.
Committed (r9789), thanks.
> I think I understood what the Xs Ym mean...
> Xs: X "scalar" arguments
> Ym: Y "memory" (pointer) arguments.
I explained that a little better in the comments, too.
Nick
|