Re: [V1][PATCH 2/3] Use the element_count atribute info in builtin object size [PR108896].
> On May 27, 2023, at 6:20 AM, Martin Uecker wrote:
>
>
> Thank you for working on this!
>
>
> Here are a couple of comments:
>
> How is the size for an object with FAM defined?
Right now, with the attribute approach, the sizeof for the object with FAM is
not impacted, and kept the same as before without this attribute.
The information carried from the attribute only is used in _builtin_object_size
and bound sanitizer.
Doing this will help mitigating the existing source code to use this new
information easier.
If later this feature is promoted to C standard, I believe that the sizeof for
the object with FAM should be updated too. And the size information of the
FAM will be embedded into the TYPE.
>
> There are at least three possible choices:
>
> offset(..) + N * sizeof
> sizeof(..) + N * sizeof
> or the size of a struct with the replacement array.
>
> Or is this not relevant here?
>
>
> I would personally prefer an attribute which does
> not use a string,
Using “string” is the simplest implementation to prove the feasibility of this
new attribute.
We can definitely use C expressions if we all think it’s necessary, the
implementation in the C FE will be more complicate when using
C expression I think. (When parsing the attribute, the type of the whole
structure and other fields are not available yet, so, we might need to
Delay the parsing of the attribute after the whole structure is done..)
> but uses C expressions, so that
> one could write something like this (although I would
> limit it initially to the most simple case)
>
> struct {
> struct bar { int n; }* ptr;
> int buf[] [[element_count(.ptr->n + 3)]];
> };
>
> Of course, we could still support this later even
> if we use a string now.
Looks like that CLANG’s -fbounds-safely implements this attribute by using C
expression as the argument and they added a late parse for C.
Not sure whether we should be compatible with CLANG on this, if so, we might
need to do the same.
Thanks a lot for your suggestions and comments.
Qing
>
> Martin
>
>
>
>
> Am Donnerstag, dem 25.05.2023 um 16:14 + schrieb Qing Zhao:
>> 2023-05-17 Qing Zhao
>>
>> gcc/ChangeLog:
>>
>> PR C/108896
>> * tree-object-size.cc (addr_object_size): Use the element_count
>> attribute info.
>> * tree.cc (component_ref_has_element_count_p): New function.
>> (component_ref_get_element_count): New function.
>> * tree.h (component_ref_has_element_count_p): New prototype.
>> (component_ref_get_element_count): New prototype.
>>
>> gcc/testsuite/ChangeLog:
>>
>> PR C/108896
>> * gcc.dg/flex-array-element-count-2.c: New test.
>> ---
>> .../gcc.dg/flex-array-element-count-2.c | 56 +++
>> gcc/tree-object-size.cc | 37 ++--
>> gcc/tree.cc | 93 +++
>> gcc/tree.h| 10 ++
>> 4 files changed, 189 insertions(+), 7 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/flex-array-element-count-2.c
>>
>> diff --git a/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
>> b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
>> new file mode 100644
>> index 000..5a280e8c731
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
>> @@ -0,0 +1,56 @@
>> +/* test the attribute element_count and its usage in
>> + * __builtin_dynamic_object_size. */
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include "builtin-object-size-common.h"
>> +
>> +#define expect(p, _v) do { \
>> +size_t v = _v; \
>> +if (p == v) \
>> +__builtin_printf ("ok: %s == %zd\n", #p, p); \
>> +else \
>> +{ \
>> + __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>> + FAIL (); \
>> +} \
>> +} while (0);
>> +
>> +struct flex {
>> + int b;
>> + int c[];
>> +} *array_flex;
>> +
>> +struct annotated {
>> + int b;
>> + int c[] __attribute__ ((element_count ("b")));
>> +} *array_annotated;
>> +
>> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
>> +{
>> + array_flex
>> += (struct flex *)malloc (sizeof (struct flex)
>> + + normal_count * sizeof (int));
>> + array_flex->b = normal_count;
>> +
>> + array_annotated
>> += (struct annotated *)malloc (sizeof (struct annotated)
>> ++ attr_count * sizeof (int));
>> + array_annotated->b = attr_count;
>> +
>> + return;
>> +}
>> +
>> +void __attribute__((__noinline__)) test ()
>> +{
>> +expect(__builtin_dynamic_object_size(array_flex->c, 1), -1);
>> +expect(__builtin_dynamic_object_size(array_annotated->c, 1),
>> + array_annotated->b * sizeof (int));
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + setup (10,10);
>> + test ();
>> + DONE ();
>> +}
>> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
>> index
Re: [V1][PATCH 2/3] Use the element_count atribute info in builtin object size [PR108896].
Thank you for working on this!
Here are a couple of comments:
How is the size for an object with FAM defined?
There are at least three possible choices:
offset(..) + N * sizeof
sizeof(..) + N * sizeof
or the size of a struct with the replacement array.
Or is this not relevant here?
I would personally prefer an attribute which does
not use a string, but uses C expressions, so that
one could write something like this (although I would
limit it initially to the most simple case)
struct {
struct bar { int n; }* ptr;
int buf[] [[element_count(.ptr->n + 3)]];
};
Of course, we could still support this later even
if we use a string now.
Martin
Am Donnerstag, dem 25.05.2023 um 16:14 + schrieb Qing Zhao:
> 2023-05-17 Qing Zhao
>
> gcc/ChangeLog:
>
> PR C/108896
> * tree-object-size.cc (addr_object_size): Use the element_count
> attribute info.
> * tree.cc (component_ref_has_element_count_p): New function.
> (component_ref_get_element_count): New function.
> * tree.h (component_ref_has_element_count_p): New prototype.
> (component_ref_get_element_count): New prototype.
>
> gcc/testsuite/ChangeLog:
>
> PR C/108896
> * gcc.dg/flex-array-element-count-2.c: New test.
> ---
> .../gcc.dg/flex-array-element-count-2.c | 56 +++
> gcc/tree-object-size.cc | 37 ++--
> gcc/tree.cc | 93 +++
> gcc/tree.h| 10 ++
> 4 files changed, 189 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/flex-array-element-count-2.c
>
> diff --git a/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
> b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
> new file mode 100644
> index 000..5a280e8c731
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
> @@ -0,0 +1,56 @@
> +/* test the attribute element_count and its usage in
> + * __builtin_dynamic_object_size. */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "builtin-object-size-common.h"
> +
> +#define expect(p, _v) do { \
> +size_t v = _v; \
> +if (p == v) \
> + __builtin_printf ("ok: %s == %zd\n", #p, p); \
> +else \
> + { \
> + __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
> + FAIL (); \
> + } \
> +} while (0);
> +
> +struct flex {
> + int b;
> + int c[];
> +} *array_flex;
> +
> +struct annotated {
> + int b;
> + int c[] __attribute__ ((element_count ("b")));
> +} *array_annotated;
> +
> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
> +{
> + array_flex
> += (struct flex *)malloc (sizeof (struct flex)
> ++ normal_count * sizeof (int));
> + array_flex->b = normal_count;
> +
> + array_annotated
> += (struct annotated *)malloc (sizeof (struct annotated)
> + + attr_count * sizeof (int));
> + array_annotated->b = attr_count;
> +
> + return;
> +}
> +
> +void __attribute__((__noinline__)) test ()
> +{
> +expect(__builtin_dynamic_object_size(array_flex->c, 1), -1);
> +expect(__builtin_dynamic_object_size(array_annotated->c, 1),
> +array_annotated->b * sizeof (int));
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + setup (10,10);
> + test ();
> + DONE ();
> +}
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 9a936a91983..f9aadd59054 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -585,6 +585,7 @@ addr_object_size (struct object_size_info *osi,
> const_tree ptr,
> if (pt_var != TREE_OPERAND (ptr, 0))
> {
> tree var;
> + tree element_count_ref = NULL_TREE;
>
>
> if (object_size_type & OST_SUBOBJECT)
> {
> @@ -600,11 +601,12 @@ addr_object_size (struct object_size_info *osi,
> const_tree ptr,
> var = TREE_OPERAND (var, 0);
> if (var != pt_var && TREE_CODE (var) == ARRAY_REF)
> var = TREE_OPERAND (var, 0);
> - if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
> + if (! component_ref_has_element_count_p (var)
> + && ((! TYPE_SIZE_UNIT (TREE_TYPE (var))
> || ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
> || (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
> && tree_int_cst_lt (pt_var_size,
> - TYPE_SIZE_UNIT (TREE_TYPE (var)
> + TYPE_SIZE_UNIT (TREE_TYPE (var)))
> var = pt_var;
> else if (var != pt_var && TREE_CODE (pt_var) == MEM_REF)
> {
> @@ -612,6 +614,7 @@ addr_object_size (struct object_size_info *osi,
> const_tree ptr,
> /* For >fld, compute object size if fld isn't a flexible array
> member. */
> bool is_flexible_array_mem_ref = false;
> +
> while (v && v != pt_var)
>
[V1][PATCH 2/3] Use the element_count atribute info in builtin object size [PR108896].
2023-05-17 Qing Zhao
gcc/ChangeLog:
PR C/108896
* tree-object-size.cc (addr_object_size): Use the element_count
attribute info.
* tree.cc (component_ref_has_element_count_p): New function.
(component_ref_get_element_count): New function.
* tree.h (component_ref_has_element_count_p): New prototype.
(component_ref_get_element_count): New prototype.
gcc/testsuite/ChangeLog:
PR C/108896
* gcc.dg/flex-array-element-count-2.c: New test.
---
.../gcc.dg/flex-array-element-count-2.c | 56 +++
gcc/tree-object-size.cc | 37 ++--
gcc/tree.cc | 93 +++
gcc/tree.h| 10 ++
4 files changed, 189 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/flex-array-element-count-2.c
diff --git a/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
new file mode 100644
index 000..5a280e8c731
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/flex-array-element-count-2.c
@@ -0,0 +1,56 @@
+/* test the attribute element_count and its usage in
+ * __builtin_dynamic_object_size. */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+#define expect(p, _v) do { \
+size_t v = _v; \
+if (p == v) \
+ __builtin_printf ("ok: %s == %zd\n", #p, p); \
+else \
+ { \
+ __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
+ FAIL (); \
+ } \
+} while (0);
+
+struct flex {
+ int b;
+ int c[];
+} *array_flex;
+
+struct annotated {
+ int b;
+ int c[] __attribute__ ((element_count ("b")));
+} *array_annotated;
+
+void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
+{
+ array_flex
+= (struct flex *)malloc (sizeof (struct flex)
+ + normal_count * sizeof (int));
+ array_flex->b = normal_count;
+
+ array_annotated
+= (struct annotated *)malloc (sizeof (struct annotated)
+ + attr_count * sizeof (int));
+ array_annotated->b = attr_count;
+
+ return;
+}
+
+void __attribute__((__noinline__)) test ()
+{
+expect(__builtin_dynamic_object_size(array_flex->c, 1), -1);
+expect(__builtin_dynamic_object_size(array_annotated->c, 1),
+ array_annotated->b * sizeof (int));
+}
+
+int main(int argc, char *argv[])
+{
+ setup (10,10);
+ test ();
+ DONE ();
+}
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 9a936a91983..f9aadd59054 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -585,6 +585,7 @@ addr_object_size (struct object_size_info *osi, const_tree
ptr,
if (pt_var != TREE_OPERAND (ptr, 0))
{
tree var;
+ tree element_count_ref = NULL_TREE;
if (object_size_type & OST_SUBOBJECT)
{
@@ -600,11 +601,12 @@ addr_object_size (struct object_size_info *osi,
const_tree ptr,
var = TREE_OPERAND (var, 0);
if (var != pt_var && TREE_CODE (var) == ARRAY_REF)
var = TREE_OPERAND (var, 0);
- if (! TYPE_SIZE_UNIT (TREE_TYPE (var))
+ if (! component_ref_has_element_count_p (var)
+&& ((! TYPE_SIZE_UNIT (TREE_TYPE (var))
|| ! tree_fits_uhwi_p (TYPE_SIZE_UNIT (TREE_TYPE (var)))
|| (pt_var_size && TREE_CODE (pt_var_size) == INTEGER_CST
&& tree_int_cst_lt (pt_var_size,
- TYPE_SIZE_UNIT (TREE_TYPE (var)
+ TYPE_SIZE_UNIT (TREE_TYPE (var)))
var = pt_var;
else if (var != pt_var && TREE_CODE (pt_var) == MEM_REF)
{
@@ -612,6 +614,7 @@ addr_object_size (struct object_size_info *osi, const_tree
ptr,
/* For >fld, compute object size if fld isn't a flexible array
member. */
bool is_flexible_array_mem_ref = false;
+
while (v && v != pt_var)
switch (TREE_CODE (v))
{
@@ -639,6 +642,8 @@ addr_object_size (struct object_size_info *osi, const_tree
ptr,
break;
}
is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
+ element_count_ref = component_ref_get_element_count (v);
+
while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
!= UNION_TYPE
@@ -652,8 +657,11 @@ addr_object_size (struct object_size_info *osi, const_tree
ptr,
== RECORD_TYPE)
{
/* compute object size only if v is not a
- flexible array member. */
- if (!is_flexible_array_mem_ref)
+ flexible array
