|
From: Sebastian <sc...@nb...> - 2003-04-14 14:38:09
Attachments:
valtest.tar.gz
|
Hello, I use valgrind a lot and its a really decent program. Today I stumbled across a quite weird thing which might be a bug in my program, in valgrind or in both. This is how both valgrind 1.0.4 (debian unstable) and 1.9.4 (selfcompiled) complains about code in my program. The first complaint is: ==1682== Use of uninitialised value of size 8 ==1682== at 0x804AB95: be_random_prob (utility.c:111) ==1682== by 0x804F7CB: codegen_getreg (codegen.c:165) Where the interesting part of this function is this: 0x804ab7e <be_random_prob+69>: push $0xffffffff 0x804ab80 <be_random_prob+71>: call 0x804aa89 <be_random> 0x804ab85 <be_random_prob+76>: add $0x10,%esp 0x804ab88 <be_random_prob+79>: mov %eax,0xffffffec(%ebp) 0x804ab8b <be_random_prob+82>: mov 0xffffffec(%ebp),%eax 0x804ab8e <be_random_prob+85>: mov $0x0,%edx 0x804ab93 <be_random_prob+90>: push %edx 0x804ab94 <be_random_prob+91>: push %eax 0x804ab95 <be_random_prob+92>: fildll (%esp,1) 0x804ab98 <be_random_prob+95>: lea 0x8(%esp,1),%esp 0x804ab9c <be_random_prob+99>: fstpl 0xfffffff0(%ebp) It extracted all functions taking part in this code to extra files and ran some tests, without successes. The situation is quite simple: be_random returns one random unsigned int (in %eax), %edx is zeroed out, a 64 bit integer is temporarily created on the stack and the resulting number is loaded into a double register (fildll). The code was generated by gcc 3.2.3 without any optimizations active. I attached a test program to this mail, which does _not_ reproduce the bug, but contains exactly the same code (which I believe to be correct). So, could the valgrind complaint be a side effect of an uncatched bug in my program earlier or is it possibly a bug in valgrind? (I can provide the original program triggering the bug as binary to the authors if they wish so, but not to the list). ciao, Sebastian -- -. sc...@nb... -. + http://segfault.net/~scut/ `--------------------. -' segfault.net/~scut/pgp `' 5453 AC95 1E02 FDA7 50D2 A42D 427E 6DEF 745A 8E07 `- 4 BLU-82/MOAB articles offered, payment due. hi echelon! -----------------' |
|
From: Sebastian <sc...@nb...> - 2003-04-14 14:54:39
|
Hello again, Just wanted to say that I just tested it with 1.9.5 and that produces exactly the same behaviour. ciao, Sebastian -- -. sc...@nb... -. + http://segfault.net/~scut/ `--------------------. -' segfault.net/~scut/pgp `' 5453 AC95 1E02 FDA7 50D2 A42D 427E 6DEF 745A 8E07 `- 4 BLU-82/MOAB articles offered, payment due. hi echelon! -----------------' |
|
From: Jeremy F. <je...@go...> - 2003-04-15 01:16:36
|
Quoting Sebastian <sc...@nb...>:
> It extracted all functions taking part in this code to extra files and
> ran
> some tests, without successes. The situation is quite simple:
> be_random
> returns one random unsigned int (in %eax), %edx is zeroed out, a 64
> bit
> integer is temporarily created on the stack and the resulting number
> is
> loaded into a double register (fildll).
Well, the behaviour you're seeing is consistent with be_random() returning an
undefined value in %eax. FP instructions like fildll immediately check the
defined-ness of their values (unlike integer instructions, which only check for
defined-ness when the value is used as a pointer or in a conditional
instruction). It may be that be_random() has a bug, or it is returning an
undefined value as a result of an undefined input.
There doesn't seem to be anything wrong with this particular piece of code, or
with Valgrind's behaviour.
J
|
|
From: Sebastian <sc...@nb...> - 2003-04-15 07:08:56
|
Hi,
On Mon, Apr 14, 2003 at 06:16:18PM -0700, Jeremy Fitzhardinge wrote:
> Well, the behaviour you're seeing is consistent with be_random() returning
> an undefined value in %eax. FP instructions like fildll immediately check
> the defined-ness of their values (unlike integer instructions, which only
> check for defined-ness when the value is used as a pointer or in a
> conditional instruction). It may be that be_random() has a bug, or it is
> returning an undefined value as a result of an undefined input.
unsigned int
be_random (unsigned int max)
{
unsigned int tmp;
if (rand_fd == -1)
be_randinit ();
read (rand_fd, &tmp, sizeof (tmp));
/* 0 denotes special 0 to 2^32 - 1 range
*/
if (max == 0)
return (tmp);
return (tmp % max);
}
This code looks correct to me. 'rand_fd' leads to /dev/urandom, so to
valgrind it should just look like if 'tmp' is defined by the read(), and the
return value defined by the modulo operation.
0x804aa89 <be_random>: push %ebp
0x804aa8a <be_random+1>: mov %esp,%ebp
0x804aa8c <be_random+3>: sub $0x8,%esp
0x804aa8f <be_random+6>: cmpl $0xffffffff,0x8065668
0x804aa96 <be_random+13>: jne 0x804aa9d <be_random+20>
0x804aa98 <be_random+15>: call 0x804aa4e <be_randinit>
0x804aa9d <be_random+20>: sub $0x4,%esp
0x804aaa0 <be_random+23>: push $0x4
0x804aaa2 <be_random+25>: lea 0xfffffffc(%ebp),%eax
0x804aaa5 <be_random+28>: push %eax
0x804aaa6 <be_random+29>: pushl 0x8065668
0x804aaac <be_random+35>: call 0x8048a28 <read>
0x804aab1 <be_random+40>: add $0x10,%esp
0x804aab4 <be_random+43>: cmpl $0x0,0x8(%ebp)
0x804aab8 <be_random+47>: jne 0x804aac2 <be_random+57>
0x804aaba <be_random+49>: mov 0xfffffffc(%ebp),%eax
0x804aabd <be_random+52>: mov %eax,0xfffffff8(%ebp)
0x804aac0 <be_random+55>: jmp 0x804aad0 <be_random+71>
0x804aac2 <be_random+57>: mov 0xfffffffc(%ebp),%eax
0x804aac5 <be_random+60>: mov $0x0,%edx
0x804aaca <be_random+65>: divl 0x8(%ebp)
0x804aacd <be_random+68>: mov %edx,0xfffffff8(%ebp)
0x804aad0 <be_random+71>: mov 0xfffffff8(%ebp),%eax
0x804aad3 <be_random+74>: leave
0x804aad4 <be_random+75>: ret
(Unoptimized GCC 3.2.3 code).
In any case, there is no way for be_random to return without defining %eax.
> J
ciao,
Sebastian
--
-. sc...@nb... -. + http://segfault.net/~scut/ `--------------------.
-' segfault.net/~scut/pgp `' 5453 AC95 1E02 FDA7 50D2 A42D 427E 6DEF 745A 8E07
`- 4 BLU-82/MOAB articles offered, payment due. hi echelon! -----------------'
|
|
From: Philippe E. <ph...@wa...> - 2003-04-15 18:36:41
|
Sebastian wrote:
> Hi,
>
> unsigned int
> be_random (unsigned int max)
> {
> unsigned int tmp;
>
>
> if (rand_fd == -1)
> be_randinit ();
>
> read (rand_fd, &tmp, sizeof (tmp));
>
> /* 0 denotes special 0 to 2^32 - 1 range
> */
> if (max == 0)
> return (tmp);
>
> return (tmp % max);
> }
> In any case, there is no way for be_random to return without defining
%eax.
if read fails tmp is not initialized
regards,
Philippe Elie
|
|
From: Maarten B. <maa...@mi...> - 2003-04-15 19:26:04
|
Hi,
It might be stating the obvious ... but it is always a GoodIdea(tm)
to check the return values of system calls.
Cheers,
Maarten.
On Tue, 2003-04-15 at 16:41, Philippe Elie wrote:
> Sebastian wrote:
> > Hi,
> >
>
> > unsigned int
> > be_random (unsigned int max)
> > {
> > unsigned int tmp;
> >
> >
> > if (rand_fd == -1)
> > be_randinit ();
> >
> > read (rand_fd, &tmp, sizeof (tmp));
> >
> > /* 0 denotes special 0 to 2^32 - 1 range
> > */
> > if (max == 0)
> > return (tmp);
> >
> > return (tmp % max);
> > }
>
>
> > In any case, there is no way for be_random to return without defining
> %eax.
>
> if read fails tmp is not initialized
>
> regards,
> Philippe Elie
--
Maarten Ballintijn <maa...@mi...>
Massachusetts Institute of Technology
|
|
From: Sebastian <sc...@nb...> - 2003-04-16 14:45:27
|
Hi, On Tue, Apr 15, 2003 at 03:04:20PM -0400, Maarten Ballintijn wrote: > It might be stating the obvious ... but it is always a GoodIdea(tm) to > check the return values of system calls. Args, yes, that was it. Thanks ! (I introduced the bug in two steps: First I thought read() will never fail on reading data from /dev/urandom. But then, to reproduce bug cases more exactly, I added an option to override using /dev/urandom but a fixed random data file instead. Guess what, there was more random data required than those file could provide and read() bumped against its file end). A good reason to think twice about "simply modding" existing features. Thanks to the other poster, too. > Cheers, > Maarten. ciao, Sebastian -- -. sc...@nb... -. + http://segfault.net/~scut/ `--------------------. -' segfault.net/~scut/pgp `' 5453 AC95 1E02 FDA7 50D2 A42D 427E 6DEF 745A 8E07 `- 4 BLU-82/MOAB articles offered, payment due. hi echelon! -----------------' |
|
From: Jeremy F. <je...@go...> - 2003-04-17 08:24:00
|
Quoting Sebastian <sc...@nb...>:
> This code looks correct to me. 'rand_fd' leads to /dev/urandom, so to
> valgrind it should just look like if 'tmp' is defined by the read(), and
> the
> return value defined by the modulo operation.
Only if you're sure that max is defined. Try scattering some
VALGRIND_CHECK_DEFINED()s around to see what is actually a defined value where.
BTW, this won't generate an even distribution from 0...max if that's what you
need.
J
|