diff mbox

Deal with promotions for internal functions (PR sanitizer/59399)

Message ID 20131209222440.GL11710@redhat.com
State New
Headers show

Commit Message

Marek Polacek Dec. 9, 2013, 10:24 p.m. UTC
Back in April 2011, Richard S. submitted the implementation of
internal functions [1].  It originally had this hunk of code:

 	  if (code == SSA_NAME
 	      && (g = SSA_NAME_DEF_STMT (ssa_name))
-	      && gimple_code (g) == GIMPLE_CALL)
+	      && gimple_code (g) == GIMPLE_CALL
+	      && !gimple_call_internal_p (g))
 	    pmode = promote_function_mode (type, mode, &unsignedp,
 					   TREE_TYPE
 					   (TREE_TYPE (gimple_call_fn (g))),

but the !gimple_call_internal_p (g) was turned into an assert.  This patch
turns the assert back into a condition.  That's because I actually hit that
assert with -fsanitize=signed-integer-overflow on PPC64, where we expand internal
calls at expand time.  On PPC64, integers are promoted to long when passed to
a function, that is why the assert doesn't trigger on x86_64.
There shouldn't be any harm in not calling promote_function_mode since internal
functions do not result in a call instruction.

I'm not attaching any new testcase, because that's not needed - we ICE on PPC64
with current testsuite.

Regtested/bootstrapped on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
Ok for trunk?

[1] http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00609.html

2013-12-09  Marek Polacek  <polacek@redhat.com>

	PR sanitizer/59399
	* expr.c (expand_expr_real_1): Remove assert dealing with
	internal calls and turn that into a condition instead.


	Marek

Comments

Richard Biener Dec. 10, 2013, 2:51 p.m. UTC | #1
On Mon, 9 Dec 2013, Marek Polacek wrote:

> Back in April 2011, Richard S. submitted the implementation of
> internal functions [1].  It originally had this hunk of code:
> 
>  	  if (code == SSA_NAME
>  	      && (g = SSA_NAME_DEF_STMT (ssa_name))
> -	      && gimple_code (g) == GIMPLE_CALL)
> +	      && gimple_code (g) == GIMPLE_CALL
> +	      && !gimple_call_internal_p (g))
>  	    pmode = promote_function_mode (type, mode, &unsignedp,
>  					   TREE_TYPE
>  					   (TREE_TYPE (gimple_call_fn (g))),
> 
> but the !gimple_call_internal_p (g) was turned into an assert.  This patch
> turns the assert back into a condition.  That's because I actually hit that
> assert with -fsanitize=signed-integer-overflow on PPC64, where we expand internal
> calls at expand time.  On PPC64, integers are promoted to long when passed to
> a function, that is why the assert doesn't trigger on x86_64.
> There shouldn't be any harm in not calling promote_function_mode since internal
> functions do not result in a call instruction.
> 
> I'm not attaching any new testcase, because that's not needed - we ICE on PPC64
> with current testsuite.
> 
> Regtested/bootstrapped on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
> Ok for trunk?

Looks good to me - Richard?

Thanks,
Richard.

> [1] http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00609.html
> 
> 2013-12-09  Marek Polacek  <polacek@redhat.com>
> 
> 	PR sanitizer/59399
> 	* expr.c (expand_expr_real_1): Remove assert dealing with
> 	internal calls and turn that into a condition instead.
> 
> --- gcc/expr.c.mp2	2013-12-09 18:54:16.235624592 +0100
> +++ gcc/expr.c	2013-12-09 18:55:03.580792572 +0100
> @@ -9482,13 +9482,11 @@ expand_expr_real_1 (tree exp, rtx target
>  	     the same mode we got when the variable was declared.  */
>  	  if (code == SSA_NAME
>  	      && (g = SSA_NAME_DEF_STMT (ssa_name))
> -	      && gimple_code (g) == GIMPLE_CALL)
> -	    {
> -	      gcc_assert (!gimple_call_internal_p (g));
> -	      pmode = promote_function_mode (type, mode, &unsignedp,
> -					     gimple_call_fntype (g),
> -					     2);
> -	    }
> +	      && gimple_code (g) == GIMPLE_CALL
> +	      && !gimple_call_internal_p (g))
> +	    pmode = promote_function_mode (type, mode, &unsignedp,
> +					   gimple_call_fntype (g),
> +					   2);
>  	  else
>  	    pmode = promote_decl_mode (exp, &unsignedp);
>  	  gcc_assert (GET_MODE (decl_rtl) == pmode);
> 
> 	Marek
> 
>
Richard Sandiford Dec. 10, 2013, 6:04 p.m. UTC | #2
Richard Biener <rguenther@suse.de> writes:
> On Mon, 9 Dec 2013, Marek Polacek wrote:
>
>> Back in April 2011, Richard S. submitted the implementation of
>> internal functions [1].  It originally had this hunk of code:
>> 
>>  	  if (code == SSA_NAME
>>  	      && (g = SSA_NAME_DEF_STMT (ssa_name))
>> -	      && gimple_code (g) == GIMPLE_CALL)
>> +	      && gimple_code (g) == GIMPLE_CALL
>> +	      && !gimple_call_internal_p (g))
>>  	    pmode = promote_function_mode (type, mode, &unsignedp,
>>  					   TREE_TYPE
>>  					   (TREE_TYPE (gimple_call_fn (g))),
>> 
>> but the !gimple_call_internal_p (g) was turned into an assert.  This patch
>> turns the assert back into a condition.  That's because I actually hit that
>> assert with -fsanitize=signed-integer-overflow on PPC64, where we expand internal
>> calls at expand time.  On PPC64, integers are promoted to long when passed to
>> a function, that is why the assert doesn't trigger on x86_64.
>> There shouldn't be any harm in not calling promote_function_mode since internal
>> functions do not result in a call instruction.
>> 
>> I'm not attaching any new testcase, because that's not needed - we ICE on PPC64
>> with current testsuite.
>> 
>> Regtested/bootstrapped on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
>> Ok for trunk?
>
> Looks good to me - Richard?

Yeah, fine with me too.  I suppose this wasn't ever likely to hit with
the original ARM use.

Thanks,
Richard
diff mbox

Patch

--- gcc/expr.c.mp2	2013-12-09 18:54:16.235624592 +0100
+++ gcc/expr.c	2013-12-09 18:55:03.580792572 +0100
@@ -9482,13 +9482,11 @@  expand_expr_real_1 (tree exp, rtx target
 	     the same mode we got when the variable was declared.  */
 	  if (code == SSA_NAME
 	      && (g = SSA_NAME_DEF_STMT (ssa_name))
-	      && gimple_code (g) == GIMPLE_CALL)
-	    {
-	      gcc_assert (!gimple_call_internal_p (g));
-	      pmode = promote_function_mode (type, mode, &unsignedp,
-					     gimple_call_fntype (g),
-					     2);
-	    }
+	      && gimple_code (g) == GIMPLE_CALL
+	      && !gimple_call_internal_p (g))
+	    pmode = promote_function_mode (type, mode, &unsignedp,
+					   gimple_call_fntype (g),
+					   2);
 	  else
 	    pmode = promote_decl_mode (exp, &unsignedp);
 	  gcc_assert (GET_MODE (decl_rtl) == pmode);