diff mbox

Fix PR56990

Message ID 20130422161126.GH13346@redhat.com
State New
Headers show

Commit Message

Marek Polacek April 22, 2013, 4:11 p.m. UTC 
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.


	Marek

Comments

Richard Henderson April 22, 2013, 4:19 p.m. UTC | #1 
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~
Jakub Jelinek April 22, 2013, 4:21 p.m. UTC | #2 
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
Richard Henderson April 22, 2013, 4:23 p.m. UTC | #3 
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~
diff mbox

Patch

--- 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){};
+}