Message ID | 20130422161126.GH13346@redhat.com |
---|---|
State | New |
Headers | show |
On 2013-04-22 17:11, Marek Polacek wrote: > We're getting SIGFPE, because one simply does not divide by zero. > Fixed by doing the modulo only when size != 0. > > Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8? > > 2013-04-22 Marek Polacek <polacek@redhat.com> > > PR sanitizer/56990 > * tsan.c (instrument_expr): Don't count modulo if the size > is zero. > > * gcc.dg/pr56990.c: New test. Ok. r~
On Mon, Apr 22, 2013 at 06:11:26PM +0200, Marek Polacek wrote: > We're getting SIGFPE, because one simply does not divide by zero. > Fixed by doing the modulo only when size != 0. > > Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8? > > 2013-04-22 Marek Polacek <polacek@redhat.com> > > PR sanitizer/56990 > * tsan.c (instrument_expr): Don't count modulo if the size > is zero. > > * gcc.dg/pr56990.c: New test. > > --- gcc/tsan.c.mp 2013-04-19 15:39:46.416450528 +0200 > +++ gcc/tsan.c 2013-04-22 17:23:54.115647673 +0200 > @@ -131,7 +131,8 @@ instrument_expr (gimple_stmt_iterator gs > if (TREE_READONLY (base)) > return false; > > - if (bitpos % (size * BITS_PER_UNIT) > + if ((size != 0 IMHO for size == 0 we should return false too, there is no store or read to be instrumented. So if (size == 0 || bitpos % (size * BITS_PER_UNIT) || bitsize != size * BITS_PER_UNIT) return false; ? Jakub
On 2013-04-22 17:21, Jakub Jelinek wrote: > On Mon, Apr 22, 2013 at 06:11:26PM +0200, Marek Polacek wrote: >> We're getting SIGFPE, because one simply does not divide by zero. >> Fixed by doing the modulo only when size != 0. >> >> Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.8? >> >> 2013-04-22 Marek Polacek <polacek@redhat.com> >> >> PR sanitizer/56990 >> * tsan.c (instrument_expr): Don't count modulo if the size >> is zero. >> >> * gcc.dg/pr56990.c: New test. >> >> --- gcc/tsan.c.mp 2013-04-19 15:39:46.416450528 +0200 >> +++ gcc/tsan.c 2013-04-22 17:23:54.115647673 +0200 >> @@ -131,7 +131,8 @@ instrument_expr (gimple_stmt_iterator gs >> if (TREE_READONLY (base)) >> return false; >> >> - if (bitpos % (size * BITS_PER_UNIT) >> + if ((size != 0 > > IMHO for size == 0 we should return false too, there is no store or read to > be instrumented. So > if (size == 0 > || bitpos % (size * BITS_PER_UNIT) > || bitsize != size * BITS_PER_UNIT) > return false; > ? Point. And along those lines, will we ever have bitsize == 0? If not, then just swapping the conditions is sufficient: r~
--- gcc/tsan.c.mp 2013-04-19 15:39:46.416450528 +0200 +++ gcc/tsan.c 2013-04-22 17:23:54.115647673 +0200 @@ -131,7 +131,8 @@ instrument_expr (gimple_stmt_iterator gs if (TREE_READONLY (base)) return false; - if (bitpos % (size * BITS_PER_UNIT) + if ((size != 0 + && bitpos % (size * BITS_PER_UNIT)) || bitsize != size * BITS_PER_UNIT) return false; --- gcc/testsuite/gcc.dg/pr56990.c.mp 2013-04-22 17:30:14.523876683 +0200 +++ gcc/testsuite/gcc.dg/pr56990.c 2013-04-22 17:29:06.704666252 +0200 @@ -0,0 +1,10 @@ +/* PR sanitizer/56990 */ +/* { dg-do compile { target { x86_64-*-linux* && lp64 } } } */ +/* { dg-options "-fsanitize=thread" } */ + +struct S{}; + +void foo(struct S *p) +{ + *p = (struct S){}; +}