* divide by zero in random/taviso()
@ 2014-01-17 20:21 Vince Weaver
2014-01-17 20:41 ` [patch] fix " Vince Weaver
0 siblings, 1 reply; 5+ messages in thread
From: Vince Weaver @ 2014-01-17 20:21 UTC (permalink / raw)
To: trinity
Hello
so for my perf_fuzzer I re-use the trinity random.c, and today I hit
[ 1241.026870] traps: perf_fuzzer[4509] trap divide error ip:40a779
which maps to the taviso() routine which is doing:
case 1: r = rand() % rand();
#if __WORDSIZE == 64
r <<= 32;
r |= rand() % rand();
#endif
break;
which is a divide by zero if rand() returns 0.
Is there a good way to fix this short of a lot of special case checks?
Vince Weaver
vincent.weaver@maine.edu
http://www.eece.maine.edu/~vweaver/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch] fix divide by zero in random/taviso()
2014-01-17 20:21 divide by zero in random/taviso() Vince Weaver
@ 2014-01-17 20:41 ` Vince Weaver
2014-01-17 21:03 ` Dave Jones
0 siblings, 1 reply; 5+ messages in thread
From: Vince Weaver @ 2014-01-17 20:41 UTC (permalink / raw)
To: trinity
I'm not sure if this is the cleanest fix for the issue, but it seems to
work for me.
In random.c the taviso() routine uses "rand() % rand()" which
can cause a floating point exception if rand() returns 0.
Add some tests to make sure that doesn't happen.
Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
diff --git a/random.c b/random.c
index 7def9ff..a71fcea 100644
--- a/random.c
+++ b/random.c
@@ -42,6 +42,7 @@ static unsigned long randbits(int limit)
static unsigned long taviso(void)
{
unsigned long r = 0;
+ unsigned long temp;
switch (rand() % 4) {
case 0: r = rand() & rand();
@@ -51,10 +52,14 @@ static unsigned long taviso(void)
#endif
break;
- case 1: r = rand() % rand();
+ case 1: temp = rand();
+ r = rand();
+ if (!temp) r %= temp;
#if __WORDSIZE == 64
r <<= 32;
- r |= rand() % rand();
+
+ temp = rand();
+ if (!temp) r |= rand() % temp;
#endif
break;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] fix divide by zero in random/taviso()
2014-01-17 20:41 ` [patch] fix " Vince Weaver
@ 2014-01-17 21:03 ` Dave Jones
[not found] ` <CAKB9nXvQmAU-PXjqbezf7G5bWfrjMTD5LjoFnj98K8Tyy5eQ2Q@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Dave Jones @ 2014-01-17 21:03 UTC (permalink / raw)
To: Vince Weaver; +Cc: trinity
On Fri, Jan 17, 2014 at 03:41:58PM -0500, Vince Weaver wrote:
> I'm not sure if this is the cleanest fix for the issue, but it seems to
> work for me.
Good enough.
> In random.c the taviso() routine uses "rand() % rand()" which
> can cause a floating point exception if rand() returns 0.
> Add some tests to make sure that doesn't happen.
>
> Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
>
> diff --git a/random.c b/random.c
> index 7def9ff..a71fcea 100644
> --- a/random.c
> +++ b/random.c
> @@ -42,6 +42,7 @@ static unsigned long randbits(int limit)
> static unsigned long taviso(void)
> {
> unsigned long r = 0;
> + unsigned long temp;
>
> switch (rand() % 4) {
> case 0: r = rand() & rand();
> @@ -51,10 +52,14 @@ static unsigned long taviso(void)
> #endif
> break;
>
> - case 1: r = rand() % rand();
> + case 1: temp = rand();
> + r = rand();
> + if (!temp) r %= temp;
> #if __WORDSIZE == 64
> r <<= 32;
> - r |= rand() % rand();
> +
> + temp = rand();
> + if (!temp) r |= rand() % temp;
> #endif
> break;
Good catch. Despite the function name, this is something I introduced,
not something that was in Tavis' original code in iknowthis.
Mea culpa.
Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] fix divide by zero in random/taviso()
[not found] ` <CAKB9nXvQmAU-PXjqbezf7G5bWfrjMTD5LjoFnj98K8Tyy5eQ2Q@mail.gmail.com>
@ 2014-01-21 19:07 ` Dave Jones
2014-01-21 19:47 ` Vince Weaver
0 siblings, 1 reply; 5+ messages in thread
From: Dave Jones @ 2014-01-21 19:07 UTC (permalink / raw)
To: Andrew Honig; +Cc: Vince Weaver, trinity
On Tue, Jan 21, 2014 at 11:03:43AM -0800, Andrew Honig wrote:
> > > In random.c the taviso() routine uses "rand() % rand()" which
> > > can cause a floating point exception if rand() returns 0.
> > > Add some tests to make sure that doesn't happen.
> > >
> > > Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
> > >
> > > diff --git a/random.c b/random.c
> > > index 7def9ff..a71fcea 100644
> > > --- a/random.c
> > > +++ b/random.c
> > > @@ -42,6 +42,7 @@ static unsigned long randbits(int limit)
> > > static unsigned long taviso(void)
> > > {
> > > unsigned long r = 0;
> > > + unsigned long temp;
> > >
> > > switch (rand() % 4) {
> > > case 0: r = rand() & rand();
> > > @@ -51,10 +52,14 @@ static unsigned long taviso(void)
> > > #endif
> > > break;
> > >
> > > - case 1: r = rand() % rand();
> > > + case 1: temp = rand();
> > > + r = rand();
> >
>
> Perhaps, I'm reading this wrong, but this looks backwards to me. This will
> perform the mod operation only when temp is 0. Shouldn't this be
> if (temp) r %= temp;
clang seems to agree.
random.c:57:16: warning: Division by zero
if (!temp) r %= temp;
~~^~~~~~~
random.c:62:26: warning: Division by zero
if (!temp) r |= rand() % temp;
~~~~~~~^~~~~~
Vince, want to send a follow-up fix ?
Dave
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] fix divide by zero in random/taviso()
2014-01-21 19:07 ` Dave Jones
@ 2014-01-21 19:47 ` Vince Weaver
0 siblings, 0 replies; 5+ messages in thread
From: Vince Weaver @ 2014-01-21 19:47 UTC (permalink / raw)
To: Dave Jones; +Cc: Andrew Honig, trinity
On Tue, 21 Jan 2014, Dave Jones wrote:
> random.c:57:16: warning: Division by zero
> if (!temp) r %= temp;
> ~~^~~~~~~
> random.c:62:26: warning: Division by zero
> if (!temp) r |= rand() % temp;
> ~~~~~~~^~~~~~
>
> Vince, want to send a follow-up fix ?
Yes, brown-paper bag time. Never trust "it compiles and runs" as
testing for a bug that only shows up 1 in 4 billion times anyway.
I've copied the code into a test harness and actually ran it with code
that returns zero more often to verify it works (after hacking things
so gcc didn't "helpfully" optimize the zero generating code away).
---
The previous fix to the divide-by-zero bug in random/taviso()
(fea56c28830f "fix divide by zero in random/taviso()")
had the zero-checks inverted. This meant the old div-by-zero bug
was still there, and worse, it meant that the number being generated
always had 32-bits of zeros in the bottom half on a 64-bit machine.
Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
diff --git a/random.c b/random.c
index 75fa868..38ed22f 100644
--- a/random.c
+++ b/random.c
@@ -54,12 +54,12 @@ static unsigned long taviso(void)
case 1: temp = rand();
r = rand();
- if (!temp) r %= temp;
+ if (temp) r %= temp;
#if __WORDSIZE == 64
r <<= 32;
temp = rand();
- if (!temp) r |= rand() % temp;
+ if (temp) r |= rand() % temp;
#endif
break;
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-21 19:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-17 20:21 divide by zero in random/taviso() Vince Weaver
2014-01-17 20:41 ` [patch] fix " Vince Weaver
2014-01-17 21:03 ` Dave Jones
[not found] ` <CAKB9nXvQmAU-PXjqbezf7G5bWfrjMTD5LjoFnj98K8Tyy5eQ2Q@mail.gmail.com>
2014-01-21 19:07 ` Dave Jones
2014-01-21 19:47 ` Vince Weaver
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.