|
From: Tom H. <th...@cy...> - 2004-02-28 03:09:06
|
Nightly build on standard ( Red Hat 7.2 ) started at 2004-02-28 03:00:02 GMT Checking out source tree ... done Configuring ... done Building ... done Running regression tests ... done Last 20 lines of log.verbose follow memcheck/tests/pushfpopf (stderr) memcheck/tests/realloc1 (stderr) memcheck/tests/realloc2 (stderr) memcheck/tests/realloc3 (stderr) memcheck/tests/sigaltstack (stderr) memcheck/tests/signal2 (stderr) memcheck/tests/supp1 (stderr) memcheck/tests/supp2 (stderr) memcheck/tests/suppfree (stderr) memcheck/tests/threadederrno (stderr) memcheck/tests/trivialleak (stderr) memcheck/tests/tronical (stderr) memcheck/tests/weirdioctl (stderr) memcheck/tests/writev (stderr) memcheck/tests/zeropage (stderr) none/tests/pth_blockedsig (stdout) none/tests/pth_blockedsig (stderr) none/tests/yield (stdout) none/tests/yield (stderr) |
|
From: Tom H. <th...@cy...> - 2004-02-28 18:16:03
Attachments:
valgrind-glibc22-patch
|
In message <E1A...@st...>
Tom Hughes <th...@cy...> wrote:
> Last 20 lines of log.verbose follow
>
> memcheck/tests/pushfpopf (stderr)
> memcheck/tests/realloc1 (stderr)
> memcheck/tests/realloc2 (stderr)
> memcheck/tests/realloc3 (stderr)
> memcheck/tests/sigaltstack (stderr)
> memcheck/tests/signal2 (stderr)
> memcheck/tests/supp1 (stderr)
> memcheck/tests/supp2 (stderr)
> memcheck/tests/suppfree (stderr)
> memcheck/tests/threadederrno (stderr)
> memcheck/tests/trivialleak (stderr)
> memcheck/tests/tronical (stderr)
> memcheck/tests/weirdioctl (stderr)
> memcheck/tests/writev (stderr)
> memcheck/tests/zeropage (stderr)
> none/tests/pth_blockedsig (stdout)
> none/tests/pth_blockedsig (stderr)
> none/tests/yield (stdout)
> none/tests/yield (stderr)
There are a number of failures here of this form:
***************
*** 2 ****
--- 3,7 ----
+ valgrind's libpthread.so: init_libc_tsd_keys: lock
+ Please report this bug at: valgrind.kde.org
Which is a bit odd, because from looking at the code it means that
the attempt to lock the mutex in init_libc_tsd_keys is failing for
some reason.
There are also a number that look something like this:
***************
*** 5 ****
! at 0x........: fcntl (in /...libc...)
--- 5 ----
! at 0x........: fcntl (../sysdeps/unix/sysv/linux/i386/fcntl.c:49)
Which are presumably caused by the C library having debugging symbols
in it for some reason.
Some are caused by a couple of rules being commented out in glibc-2.2.supp
for some reason - possibly due to a typo in the rules. A patch for that is
attached - it also adds a couple of new suppressions that are needed.
There is also a new assertion firing in some tests:
+ valgrind: vg_libpthread.c:... (pthread_cond_init): Assertion `sizeof(*cond) >=
sizeof(vg_pthread_cond_t)' failed.
Presumably pthread_cond_t is a bit smaller in glibc 2.2 or something.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Tom H. <th...@cy...> - 2004-02-28 18:19:57
|
In message <0aa...@lo...>
Tom Hughes <th...@cy...> wrote:
> There is also a new assertion firing in some tests:
>
> + valgrind: vg_libpthread.c:... (pthread_cond_init): Assertion `sizeof(*cond) >=
> sizeof(vg_pthread_cond_t)' failed.
>
> Presumably pthread_cond_t is a bit smaller in glibc 2.2 or something.
Indeed. Having checked, it looks like the padding was only added
to pthread_cond_t in recent glibc's. On RH7.2 and 7.3 we have this:
/* Conditions (not abstract because of PTHREAD_COND_INITIALIZER */
typedef struct
{
struct _pthread_fastlock __c_lock; /* Protect against concurrent access */
_pthread_descr __c_waiting; /* Threads waiting on this condition */
} pthread_cond_t;
but by RH8.0 it has become this:
typedef struct
{
struct _pthread_fastlock __c_lock; /* Protect against concurrent access */
_pthread_descr __c_waiting; /* Threads waiting on this condition */
char __padding[48 - sizeof (struct _pthread_fastlock)
- sizeof (_pthread_descr) - sizeof (__pthread_cond_align_t)];
__pthread_cond_align_t __align;
} pthread_cond_t;
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Nicholas N. <nj...@ca...> - 2004-02-29 00:22:40
|
On Sat, 28 Feb 2004, Tom Hughes wrote: > The one in helgrind/tests/inherit is down to it failing to print > the "XXX We expect an error on inherit.c:48" message but I can't > quite work out where that comes from anyway. To be honest I don't > think I've ever seen that test pass anywhere. That confused me too. The test is meant to fail, the message is a "fixme" that indicates what the expected output should be, ie. Helgrind should give an error but it doesn't. The right way to handle this is to have a way of marking a test as a known failure; that way we don't get reports about it failing, but it's clearly marked as doing the wrong thing. > The fdleak_ failures are caused by the overnight tests being run from > cron and getting /dev/null as standard input which is explicitly left > alone by the filter script and hence the output doesn't match the > expected results which are designed for running from a terminal where > the terminal name would get filtered out. I understood that paragraph up to "standard input", but the rest lost me. How is the filter script interacting with /dev/null? How does the terminal name get filtered out? Sorry if I'm being dense. > + valgrind's libpthread.so: init_libc_tsd_keys: lock > + Please report this bug at: valgrind.kde.org > > Which is a bit odd, because from looking at the code it means that > the attempt to lock the mutex in init_libc_tsd_keys is failing for > some reason. Hmm, I don't know about that one. > > There is also a new assertion firing in some tests: > > > > + valgrind: vg_libpthread.c:... (pthread_cond_init): Assertion `sizeof(*cond) >= > > sizeof(vg_pthread_cond_t)' failed. > > > > Presumably pthread_cond_t is a bit smaller in glibc 2.2 or something. > > Indeed. Having checked, it looks like the padding was only added > to pthread_cond_t in recent glibc's. I've commented out the padding and committed the change. I don't imagine it will break anything, since those padding fields are unused by Valgrind. > Virtually every test on RH7.3 is failing with a series of errors > like this: > > + warning: line info addresses out of order at entry 939: 0x........ 0x........ > + warning: line info addresses out of order at entry 943: 0x........ 0x........ We could remove such lines using the basic filter. Or not spit out the warning if it's not important. I think all the other problems could be handled by allowing multiple expected outputs, as Julian suggested. I'll look into changing the script. The only question is if you have multiple acceptable outputs, if none of them match, which do you use to form the .diff file? I figure it can give a .diff for every acceptable output, eg. .diff, .diff2, .diff3... N |
|
From: Tom H. <th...@cy...> - 2004-02-29 11:00:53
|
In message <Pin...@re...>
Nicholas Nethercote <nj...@ca...> wrote:
> On Sat, 28 Feb 2004, Tom Hughes wrote:
>
> > The fdleak_ failures are caused by the overnight tests being run from
> > cron and getting /dev/null as standard input which is explicitly left
> > alone by the filter script and hence the output doesn't match the
> > expected results which are designed for running from a terminal where
> > the terminal name would get filtered out.
>
> I understood that paragraph up to "standard input", but the rest lost me.
> How is the filter script interacting with /dev/null? How does the
> terminal name get filtered out? Sorry if I'm being dense.
The /dev/null was actually a red herring as standard input is
actually a pipe under cron it seems.
If you run the fdleak tests from an interactive session then the
output will include something like this:
==12551== Open file descriptor 2: /dev/pts/13
==12551== <inherited from parent>
==12551==
==12551== Open file descriptor 1: /dev/pts/13
==12551== <inherited from parent>
==12551==
==12551== Open file descriptor 0: /dev/pts/13
==12551== <inherited from parent>
which is then stripped to this:
==12551== Open file descriptor .: .
==12551== <inherited from parent>
==12551==
==12551== Open file descriptor .: .
==12551== <inherited from parent>
==12551==
==12551== Open file descriptor .: .
==12551== <inherited from parent>
So that the exact name of the terminal device doesn't effect whether
or not the test passes. Running from cron means that the standard input
output and error devices are pipes instead of terminals, which means
you get output like this:
==12509== Open file descriptor 2:
==12509== <inherited from parent>
==12509==
==12509== Open file descriptor 1:
==12509== <inherited from parent>
==12509==
==12509== Open file descriptor 0:
==12509== <inherited from parent>
Note that valgrind isn't able to work out what the descriptor is
because it's a pipe, so leaves that blank. That is then stripped
to this:
==12509== Open file descriptor .:
==12509== <inherited from parent>
==12509==
==12509== Open file descriptor .:
==12509== <inherited from parent>
==12509==
==12509== Open file descriptor .:
==12509== <inherited from parent>
Which doesn't match the results because of the final . being missing.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Tom H. <th...@cy...> - 2004-02-29 13:07:22
|
In message <Pin...@re...>
Nicholas Nethercote <nj...@ca...> wrote:
> On Sat, 28 Feb 2004, Tom Hughes wrote:
>
> > Virtually every test on RH7.3 is failing with a series of errors
> > like this:
> >
> > + warning: line info addresses out of order at entry 939: 0x........ 0x........
> > + warning: line info addresses out of order at entry 943: 0x........ 0x........
>
> We could remove such lines using the basic filter. Or not spit out the
> warning if it's not important.
I've filtered them out for now so I can see the wood from the trees.
Somebody with more knowledge of the stabs code than me needs to work
out what the messages mean and whether they're important.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Jeremy F. <je...@go...> - 2004-02-29 21:51:17
|
On Sat, 2004-02-28 at 10:13, Tom Hughes wrote: > *************** > *** 2 **** > --- 3,7 ---- > + valgrind's libpthread.so: init_libc_tsd_keys: lock > + Please report this bug at: valgrind.kde.org > > Which is a bit odd, because from looking at the code it means that > the attempt to lock the mutex in init_libc_tsd_keys is failing for > some reason. Yes, this is the result of some unwanted recursion. Um. What was it. Ah yes: my_malloc ends up calling real malloc(), which on some libc's ends up calling init_libc_tsd_keys recursively. The mutex is already held, so it fails the second time through. my_malloc could use the MALLOC client request, but that causes other problems, which I don't remember completely. Some dependency on whether the tool you have is overriding malloc or not. J |
|
From: Jeremy F. <je...@go...> - 2004-02-29 23:39:55
|
On Sun, 2004-02-29 at 14:16, Tom Hughes wrote:
> In message <1078091257.30493.82.camel@localhost.localdomain>
> Jeremy Fitzhardinge <je...@go...> wrote:
>
> > my_malloc ends up calling real malloc(), which on some libc's ends up
> > calling init_libc_tsd_keys recursively. The mutex is already held, so
> > it fails the second time through.
>
> Doesn't init_libc_tsd_keys already check for recursive calls before
> trying to do the lock though?
Not correctly. The recursion path is:
init_libc_tsd_keys ->
__pthread_key_create ->
get_or_allocate_specifics_ptr ->
my_malloc ->
malloc ->
... ->
init_libc_tsd_keys
init_libc_tsd_keys doesn't set libc_specifics_inited until after calling
__pthread_key_create.
Perhaps it should set it as early as possible.
J
|
|
From: Tom H. <th...@cy...> - 2004-03-07 13:47:43
Attachments:
valgrind-specifics-patch
|
In message <1078097773.30493.134.camel@localhost.localdomain>
Jeremy Fitzhardinge <je...@go...> wrote:
> On Sun, 2004-02-29 at 14:16, Tom Hughes wrote:
> > In message <1078091257.30493.82.camel@localhost.localdomain>
> > Jeremy Fitzhardinge <je...@go...> wrote:
> >
> > > my_malloc ends up calling real malloc(), which on some libc's ends up
> > > calling init_libc_tsd_keys recursively. The mutex is already held, so
> > > it fails the second time through.
> >
> > Doesn't init_libc_tsd_keys already check for recursive calls before
> > trying to do the lock though?
>
> Not correctly. The recursion path is:
>
> init_libc_tsd_keys ->
> __pthread_key_create ->
> get_or_allocate_specifics_ptr ->
> my_malloc ->
> malloc ->
> ... ->
> init_libc_tsd_keys
>
> init_libc_tsd_keys doesn't set libc_specifics_inited until after calling
> __pthread_key_create.
>
> Perhaps it should set it as early as possible.
That wouldn't really solve the problem though. The problem is that
valrind's libpthread is trying to use malloc to allocate space for
the libc specific's, but at least one those specifics is used by
the libc malloc which just can't work...
If you look at glibc you'll see that it doesn't use the general
pthread thread specific data system to handle the libc specific
data, presumably because of this malloc issue. Instead it has a
small fixed arrary of libc specific pointers in each thread's
descriptor.
I've created a patch to move the libc specifics into the thread
state array that I created to handle errno etc so that there is
no need to use malloc, and that seems to solve the problem.
The patch is attached, and I'll commit it later on unless somebody
spots a problem with it.
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|
|
From: Julian S. <js...@ac...> - 2004-02-29 01:03:52
|
> I think all the other problems could be handled by allowing multiple > expected outputs, as Julian suggested. I'll look into changing the > script. The only question is if you have multiple acceptable outputs, if > none of them match, which do you use to form the .diff file? I figure > it can give a .diff for every acceptable output, eg. .diff, .diff2, > .diff3... I'd vote for that, viz, one .diff file for each acceptable output. J |
|
From: Nicholas N. <nj...@ca...> - 2004-02-29 01:34:48
|
On Sun, 29 Feb 2004, Julian Seward wrote: > > I think all the other problems could be handled by allowing multiple > > expected outputs, as Julian suggested. I'll look into changing the > > script. The only question is if you have multiple acceptable outputs, if > > none of them match, which do you use to form the .diff file? I figure > > it can give a .diff for every acceptable output, eg. .diff, .diff2, > > .diff3... > > I'd vote for that, viz, one .diff file for each acceptable output. Done. N |
|
From: Tom H. <th...@cy...> - 2004-02-29 22:20:46
|
In message <1078091257.30493.82.camel@localhost.localdomain>
Jeremy Fitzhardinge <je...@go...> wrote:
> my_malloc ends up calling real malloc(), which on some libc's ends up
> calling init_libc_tsd_keys recursively. The mutex is already held, so
> it fails the second time through.
Doesn't init_libc_tsd_keys already check for recursive calls before
trying to do the lock though?
Tom
--
Tom Hughes (th...@cy...)
Software Engineer, Cyberscience Corporation
http://www.cyberscience.com/
|