Patchwork Fix PR C++/50970 -- Function pointer dereferenced twice in if statement on Arm cpu

login
register
mail settings
Submitter Zhenqiang Chen
Date Sept. 20, 2012, 7:51 a.m.
Message ID <CACgzC7AboXhrnJEcssAtnWFQryTn3zzKW=LVH3PgwDp8+gHeQg@mail.gmail.com>
Download mbox | patch
Permalink /patch/185354/
State New
Headers show

Comments

Zhenqiang Chen - Sept. 20, 2012, 7:51 a.m.
Hi,

PR 50970 is a general c++ front-end issue for targets which define
TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta, although the
reporter had issues only on ARM.

Root cause: It misses a check for side effects when generating pfn and
delta related expressions.

In the failed case, op0 is a function call. pfn0 and delta0 are
expressions based on the return value of op0. Without the check, the
function will be called twice.

The patch is to add the check for op0.

No make check regression on ARM.

Is it OK for trunk?

Thanks!
-Zhenqiang

cp/ChangeLog:
2012-09-20  Zhenqiang Chen <zhenqiang.chen@linaro.org>

	PR c++/50970
	* typeck.c (cp_build_binary_op): Check side effects before generating
	pfn and delta related expressions.
Richard Earnshaw - Sept. 21, 2012, 10:42 p.m.
On 20 Sep 2012, at 08:51, "Zhenqiang Chen" <zhenqiang.chen@linaro.org> wrote:

> Hi,
> 
> PR 50970 is a general c++ front-end issue for targets which define
> TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta, although the
> reporter had issues only on ARM.
> 
> Root cause: It misses a check for side effects when generating pfn and
> delta related expressions.
> 
> In the failed case, op0 is a function call. pfn0 and delta0 are
> expressions based on the return value of op0. Without the check, the
> function will be called twice.
> 
> The patch is to add the check for op0.
> 
> No make check regression on ARM.
> 
> Is it OK for trunk?
> 
> Thanks!
> -Zhenqiang
> 
> cp/ChangeLog:
> 2012-09-20  Zhenqiang Chen <zhenqiang.chen@linaro.org>
> 
>    PR c++/50970
>    * typeck.c (cp_build_binary_op): Check side effects before generating
>    pfn and delta related expressions.
> 

Ok.

R.

> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index ad4b090..884f7d0 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -4159,18 +4159,23 @@ cp_build_binary_op (location_t location,
>      if (TARGET_PTRMEMFUNC_VBIT_LOCATION
>          == ptrmemfunc_vbit_in_delta)
>        {
> -          tree pfn0 = pfn_from_ptrmemfunc (op0);
> -          tree delta0 = delta_from_ptrmemfunc (op0);
> -          tree e1 = cp_build_binary_op (location,
> -                        EQ_EXPR,
> -                              pfn0,    
> -                              build_zero_cst (TREE_TYPE (pfn0)),
> -                        complain);
> -          tree e2 = cp_build_binary_op (location,
> -                        BIT_AND_EXPR,
> -                        delta0,
> -                            integer_one_node,
> -                        complain);
> +          tree pfn0, delta0, e1, e2;
> +
> +          if (TREE_SIDE_EFFECTS (op0))
> +        op0 = save_expr (op0);
> +
> +          pfn0 = pfn_from_ptrmemfunc (op0);
> +          delta0 = delta_from_ptrmemfunc (op0);
> +          e1 = cp_build_binary_op (location,
> +                       EQ_EXPR,
> +                         pfn0,
> +                       build_zero_cst (TREE_TYPE (pfn0)),
> +                       complain);
> +          e2 = cp_build_binary_op (location,
> +                       BIT_AND_EXPR,
> +                       delta0,
> +                       integer_one_node,
> +                       complain);
>    
>          if ((complain & tf_warning)
>          && c_inhibit_evaluation_warnings == 0
>
Zhenqiang Chen - Sept. 25, 2012, 7:04 a.m.
On 22 September 2012 06:42,  <rearnsha@arm.com> wrote:
> On 20 Sep 2012, at 08:51, "Zhenqiang Chen" <zhenqiang.chen@linaro.org> wrote:
>
>> Hi,
>>
>> PR 50970 is a general c++ front-end issue for targets which define
>> TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta, although the
>> reporter had issues only on ARM.
>>
>> Root cause: It misses a check for side effects when generating pfn and
>> delta related expressions.
>>
>> In the failed case, op0 is a function call. pfn0 and delta0 are
>> expressions based on the return value of op0. Without the check, the
>> function will be called twice.
>>
>> The patch is to add the check for op0.
>>
>> No make check regression on ARM.
>>
>> Is it OK for trunk?
>>
>> Thanks!
>> -Zhenqiang
>>
>> cp/ChangeLog:
>> 2012-09-20  Zhenqiang Chen <zhenqiang.chen@linaro.org>
>>
>>    PR c++/50970
>>    * typeck.c (cp_build_binary_op): Check side effects before generating
>>    pfn and delta related expressions.
>>
>
> Ok.
>
> R.

Thanks, committed to trunk.

The bug also happens in FSF 4.7. Can I backport it to 4.7?

Thanks!
-Zhenqiang


>> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>> index ad4b090..884f7d0 100644
>> --- a/gcc/cp/typeck.c
>> +++ b/gcc/cp/typeck.c
>> @@ -4159,18 +4159,23 @@ cp_build_binary_op (location_t location,
>>      if (TARGET_PTRMEMFUNC_VBIT_LOCATION
>>          == ptrmemfunc_vbit_in_delta)
>>        {
>> -          tree pfn0 = pfn_from_ptrmemfunc (op0);
>> -          tree delta0 = delta_from_ptrmemfunc (op0);
>> -          tree e1 = cp_build_binary_op (location,
>> -                        EQ_EXPR,
>> -                              pfn0,
>> -                              build_zero_cst (TREE_TYPE (pfn0)),
>> -                        complain);
>> -          tree e2 = cp_build_binary_op (location,
>> -                        BIT_AND_EXPR,
>> -                        delta0,
>> -                            integer_one_node,
>> -                        complain);
>> +          tree pfn0, delta0, e1, e2;
>> +
>> +          if (TREE_SIDE_EFFECTS (op0))
>> +        op0 = save_expr (op0);
>> +
>> +          pfn0 = pfn_from_ptrmemfunc (op0);
>> +          delta0 = delta_from_ptrmemfunc (op0);
>> +          e1 = cp_build_binary_op (location,
>> +                       EQ_EXPR,
>> +                         pfn0,
>> +                       build_zero_cst (TREE_TYPE (pfn0)),
>> +                       complain);
>> +          e2 = cp_build_binary_op (location,
>> +                       BIT_AND_EXPR,
>> +                       delta0,
>> +                       integer_one_node,
>> +                       complain);
>>
>>          if ((complain & tf_warning)
>>          && c_inhibit_evaluation_warnings == 0
>>
>

Patch

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index ad4b090..884f7d0 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4159,18 +4159,23 @@  cp_build_binary_op (location_t location,
 	  if (TARGET_PTRMEMFUNC_VBIT_LOCATION
 	      == ptrmemfunc_vbit_in_delta)
 	    {
-	      tree pfn0 = pfn_from_ptrmemfunc (op0);
-	      tree delta0 = delta_from_ptrmemfunc (op0);
-	      tree e1 = cp_build_binary_op (location,
-					    EQ_EXPR,
-	  			            pfn0,	
-				      	    build_zero_cst (TREE_TYPE (pfn0)),
-					    complain);
-	      tree e2 = cp_build_binary_op (location,
-					    BIT_AND_EXPR,
-					    delta0,
-				            integer_one_node,
-					    complain);
+	      tree pfn0, delta0, e1, e2;
+
+	      if (TREE_SIDE_EFFECTS (op0))
+		op0 = save_expr (op0);
+
+	      pfn0 = pfn_from_ptrmemfunc (op0);
+	      delta0 = delta_from_ptrmemfunc (op0);
+	      e1 = cp_build_binary_op (location,
+				       EQ_EXPR,
+	  			       pfn0,
+				       build_zero_cst (TREE_TYPE (pfn0)),
+				       complain);
+	      e2 = cp_build_binary_op (location,
+				       BIT_AND_EXPR,
+				       delta0,
+				       integer_one_node,
+				       complain);
 	
 	      if ((complain & tf_warning)
 		  && c_inhibit_evaluation_warnings == 0