diff mbox series

[v2] c: Fully fold each parameter for call to .ACCESS_WITH_SIZE [PR119717]

Message ID 20250415184626.4081794-1-qing.zhao@oracle.com
State Committed
Commit 727f330f9ac661339af1121fc5c9b67dd0d35872
Headers
Series [v2] c: Fully fold each parameter for call to .ACCESS_WITH_SIZE [PR119717] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap fail Patch failed to apply

Commit Message

Qing Zhao April 15, 2025, 6:46 p.m. UTC 
  This is the 2nd version of the patch, the change is to replace "FALSE" with
"false" per Marek's comments.

C_MAYBE_CONST_EXPR is a C FE operator that will be removed by c_fully_fold.
In c_fully_fold, it assumes that operands of function calls have already
been folded. However, when we build call to .ACCESS_WITH_SIZE, all its
operands are not fully folded. therefore the C FE specific operator is
passed to middle-end.

In order to fix this issue, fully fold the parameters before building the
call to .ACCESS_WITH_SIZE.

Bootstrapped and regression tested on both x86 and aarch64.
Okay for trunk?

Thanks.

Qing

=====

	PR c/119717

gcc/c/ChangeLog:

	* c-typeck.cc (build_access_with_size_for_counted_by): Fully fold the
	parameters for call to .ACCESS_WITH_SIZE.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr119717.c: New test.
---
 gcc/c/c-typeck.cc               |  8 ++++++--
 gcc/testsuite/gcc.dg/pr119717.c | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr119717.c

Comments

Marek Polacek April 15, 2025, 6:56 p.m. UTC | #1 
On Tue, Apr 15, 2025 at 06:46:26PM +0000, Qing Zhao wrote:
> This is the 2nd version of the patch, the change is to replace "FALSE" with
> "false" per Marek's comments.
> 
> C_MAYBE_CONST_EXPR is a C FE operator that will be removed by c_fully_fold.
> In c_fully_fold, it assumes that operands of function calls have already
> been folded. However, when we build call to .ACCESS_WITH_SIZE, all its
> operands are not fully folded. therefore the C FE specific operator is
> passed to middle-end.
> 
> In order to fix this issue, fully fold the parameters before building the
> call to .ACCESS_WITH_SIZE.
> 
> Bootstrapped and regression tested on both x86 and aarch64.
> Okay for trunk?

LGTM now, thanks.
 
> Thanks.
> 
> Qing
> 
> =====
> 
> 	PR c/119717
> 
> gcc/c/ChangeLog:
> 
> 	* c-typeck.cc (build_access_with_size_for_counted_by): Fully fold the
> 	parameters for call to .ACCESS_WITH_SIZE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/pr119717.c: New test.
> ---
>  gcc/c/c-typeck.cc               |  8 ++++++--
>  gcc/testsuite/gcc.dg/pr119717.c | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr119717.c
> 
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 3870e8a1558..55d896e02df 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -3013,12 +3013,16 @@ build_access_with_size_for_counted_by (location_t loc, tree ref,
>    gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
>    /* The result type of the call is a pointer to the flexible array type.  */
>    tree result_type = c_build_pointer_type (TREE_TYPE (ref));
> +  tree first_param
> +    = c_fully_fold (array_to_pointer_conversion (loc, ref), false, NULL);
> +  tree second_param
> +    = c_fully_fold (counted_by_ref, false, NULL);
>  
>    tree call
>      = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
>  				    result_type, 6,
> -				    array_to_pointer_conversion (loc, ref),
> -				    counted_by_ref,
> +				    first_param,
> +				    second_param,
>  				    build_int_cst (integer_type_node, 1),
>  				    build_int_cst (counted_by_type, 0),
>  				    build_int_cst (integer_type_node, -1),
> diff --git a/gcc/testsuite/gcc.dg/pr119717.c b/gcc/testsuite/gcc.dg/pr119717.c
> new file mode 100644
> index 00000000000..e5eedc567b3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr119717.c
> @@ -0,0 +1,24 @@
> +/* PR c/119717  */
> +/* { dg-additional-options "-std=c23" } */
> +/* { dg-do compile } */
> +
> +struct annotated {
> +  unsigned count;
> +  [[gnu::counted_by(count)]] char array[];
> +};
> +
> +[[gnu::noinline,gnu::noipa]]
> +static unsigned
> +size_of (bool x, struct annotated *a)
> +{
> +  char *p = (x ? a : 0)->array;
> +  return __builtin_dynamic_object_size (p, 1);
> +}
> +
> +int main()
> +{
> +  struct annotated *p = __builtin_malloc(sizeof *p);
> +  p->count = 0;
> +  __builtin_printf ("the bdos whole is %ld\n", size_of (0, p));
> +  return 0;
> +}
> -- 
> 2.31.1
> 

Marek
Qing Zhao April 15, 2025, 8:41 p.m. UTC | #2 
Thanks.

Pushed to trunk.

Qing

> On Apr 15, 2025, at 14:56, Marek Polacek <polacek@redhat.com> wrote:
> 
> On Tue, Apr 15, 2025 at 06:46:26PM +0000, Qing Zhao wrote:
>> This is the 2nd version of the patch, the change is to replace "FALSE" with
>> "false" per Marek's comments.
>> 
>> C_MAYBE_CONST_EXPR is a C FE operator that will be removed by c_fully_fold.
>> In c_fully_fold, it assumes that operands of function calls have already
>> been folded. However, when we build call to .ACCESS_WITH_SIZE, all its
>> operands are not fully folded. therefore the C FE specific operator is
>> passed to middle-end.
>> 
>> In order to fix this issue, fully fold the parameters before building the
>> call to .ACCESS_WITH_SIZE.
>> 
>> Bootstrapped and regression tested on both x86 and aarch64.
>> Okay for trunk?
> 
> LGTM now, thanks.
> 
>> Thanks.
>> 
>> Qing
>> 
>> =====
>> 
>> PR c/119717
>> 
>> gcc/c/ChangeLog:
>> 
>> * c-typeck.cc (build_access_with_size_for_counted_by): Fully fold the
>> parameters for call to .ACCESS_WITH_SIZE.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> * gcc.dg/pr119717.c: New test.
>> ---
>> gcc/c/c-typeck.cc               |  8 ++++++--
>> gcc/testsuite/gcc.dg/pr119717.c | 24 ++++++++++++++++++++++++
>> 2 files changed, 30 insertions(+), 2 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/pr119717.c
>> 
>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>> index 3870e8a1558..55d896e02df 100644
>> --- a/gcc/c/c-typeck.cc
>> +++ b/gcc/c/c-typeck.cc
>> @@ -3013,12 +3013,16 @@ build_access_with_size_for_counted_by (location_t loc, tree ref,
>>   gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
>>   /* The result type of the call is a pointer to the flexible array type.  */
>>   tree result_type = c_build_pointer_type (TREE_TYPE (ref));
>> +  tree first_param
>> +    = c_fully_fold (array_to_pointer_conversion (loc, ref), false, NULL);
>> +  tree second_param
>> +    = c_fully_fold (counted_by_ref, false, NULL);
>> 
>>   tree call
>>     = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
>>     result_type, 6,
>> -     array_to_pointer_conversion (loc, ref),
>> -     counted_by_ref,
>> +     first_param,
>> +     second_param,
>>     build_int_cst (integer_type_node, 1),
>>     build_int_cst (counted_by_type, 0),
>>     build_int_cst (integer_type_node, -1),
>> diff --git a/gcc/testsuite/gcc.dg/pr119717.c b/gcc/testsuite/gcc.dg/pr119717.c
>> new file mode 100644
>> index 00000000000..e5eedc567b3
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr119717.c
>> @@ -0,0 +1,24 @@
>> +/* PR c/119717  */
>> +/* { dg-additional-options "-std=c23" } */
>> +/* { dg-do compile } */
>> +
>> +struct annotated {
>> +  unsigned count;
>> +  [[gnu::counted_by(count)]] char array[];
>> +};
>> +
>> +[[gnu::noinline,gnu::noipa]]
>> +static unsigned
>> +size_of (bool x, struct annotated *a)
>> +{
>> +  char *p = (x ? a : 0)->array;
>> +  return __builtin_dynamic_object_size (p, 1);
>> +}
>> +
>> +int main()
>> +{
>> +  struct annotated *p = __builtin_malloc(sizeof *p);
>> +  p->count = 0;
>> +  __builtin_printf ("the bdos whole is %ld\n", size_of (0, p));
>> +  return 0;
>> +}
>> -- 
>> 2.31.1
>> 
> 
> Marek
diff mbox series

Patch

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 3870e8a1558..55d896e02df 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -3013,12 +3013,16 @@  build_access_with_size_for_counted_by (location_t loc, tree ref,
   gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
   /* The result type of the call is a pointer to the flexible array type.  */
   tree result_type = c_build_pointer_type (TREE_TYPE (ref));
+  tree first_param
+    = c_fully_fold (array_to_pointer_conversion (loc, ref), false, NULL);
+  tree second_param
+    = c_fully_fold (counted_by_ref, false, NULL);
 
   tree call
     = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
 				    result_type, 6,
-				    array_to_pointer_conversion (loc, ref),
-				    counted_by_ref,
+				    first_param,
+				    second_param,
 				    build_int_cst (integer_type_node, 1),
 				    build_int_cst (counted_by_type, 0),
 				    build_int_cst (integer_type_node, -1),
diff --git a/gcc/testsuite/gcc.dg/pr119717.c b/gcc/testsuite/gcc.dg/pr119717.c
new file mode 100644
index 00000000000..e5eedc567b3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr119717.c
@@ -0,0 +1,24 @@ 
+/* PR c/119717  */
+/* { dg-additional-options "-std=c23" } */
+/* { dg-do compile } */
+
+struct annotated {
+  unsigned count;
+  [[gnu::counted_by(count)]] char array[];
+};
+
+[[gnu::noinline,gnu::noipa]]
+static unsigned
+size_of (bool x, struct annotated *a)
+{
+  char *p = (x ? a : 0)->array;
+  return __builtin_dynamic_object_size (p, 1);
+}
+
+int main()
+{
+  struct annotated *p = __builtin_malloc(sizeof *p);
+  p->count = 0;
+  __builtin_printf ("the bdos whole is %ld\n", size_of (0, p));
+  return 0;
+}