On 2023-08-25 11:24, Qing Zhao wrote:
This is the 3rd version of the patch, per our discussion based on the
review comments for the 1st and 2nd version, the major changes in this
version are:
Hi Qing,
I hope the review was helpful. Overall, a couple of things to consider:
1. How would you handle potential reordering between assignment of the
size to the counted_by field with the __bdos call that may consume it?
You'll probably need to express some kind of dependency there or in the
worst case, insert a barrier to disallow reordering.
2. How would you handle signedness of the size field? The size gets
converted to sizetype everywhere it is used and overflows/underflows may
produce interesting results. Do you want to limit the types to unsigned
or do you want to add a disclaimer in the docs? The former seems like
the *right* thing to do given that it is a new feature; best to enforce
the cleaner habit at the outset.
Thanks,
Sid
***Against 1st version:
1. change the name "element_count" to "counted_by";
2. change the parameter for the attribute from a STRING to an
Identifier;
3. Add logic and testing cases to handle anonymous structure/unions;
4. Clarify documentation to permit the situation when the allocation
size is larger than what's specified by "counted_by", at the same time,
it's user's error if allocation size is smaller than what's specified by
"counted_by";
5. Add a complete testing case for using counted_by attribute in
__builtin_dynamic_object_size when there is mismatch between the
allocation size and the value of "counted_by", the expecting behavior
for each case and the explanation on why in the comments.
***Against 2rd version:
1. Identify a tree node sharing issue and fixed it in the routine
"component_ref_get_counted_ty" of tree.cc;
2. Update the documentation and testing cases with the clear usage
of the fomula to compute the allocation size:
MAX (sizeof (struct A), offsetof (struct A, array[0]) + counted_by *
sizeof(element))
(the algorithm used in tree-object-size.cc is correct).
In this set of patches, the major functionality provided is:
1. a new attribute "counted_by";
2. use this new attribute in bound sanitizer;
3. use this new attribute in dynamic object size for subobject size;
As discussed, I plan to add two more separate patches sets after this initial
patch set is approved and committed.
set 1. A new warning option and a new sanitizer option for the user error
when the allocation size is smaller than the value of "counted_by".
set 2. An improvement to __builtin_dynamic_object_size for whole-object
size of the structure with FAM annaoted with counted_by.
there are also some existing bugs in tree-object-size.cc identified
during the study, and PRs were filed to record them. these bugs will
be fixed seperately with individual patches:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111030
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111040
Bootstrapped and regression tested on both aarch64 and X86, no issue.
Please see more details on the description of this work on:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/619708.html
and more discussions on
https://gcc.gnu.org/pipermail/gcc-patches/2023-August/626376.html
Okay for committing?
thanks.
Qing
Qing Zhao (3):
Provide counted_by attribute to flexible array member field (PR108896)
Use the counted_by atribute info in builtin object size [PR108896]
Use the counted_by attribute information in bound sanitizer[PR108896]
gcc/c-family/c-attribs.cc | 54 ++++-
gcc/c-family/c-common.cc | 13 ++
gcc/c-family/c-common.h | 1 +
gcc/c-family/c-ubsan.cc | 16 ++
gcc/c/c-decl.cc | 79 +++++--
gcc/doc/extend.texi | 77 +++++++
.../gcc.dg/flex-array-counted-by-2.c | 74 ++++++
.../gcc.dg/flex-array-counted-by-3.c | 210 ++++++++++++++++++
gcc/testsuite/gcc.dg/flex-array-counted-by.c | 40 ++++
.../ubsan/flex-array-counted-by-bounds-2.c | 27 +++
.../ubsan/flex-array-counted-by-bounds.c | 46 ++++
gcc/tree-object-size.cc | 37 ++-
gcc/tree.cc | 133 +++++++++++
gcc/tree.h | 15 ++
14 files changed, 797 insertions(+), 25 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-3.c
create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by.c
create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds-2.c
create mode 100644 gcc/testsuite/gcc.dg/ubsan/flex-array-counted-by-bounds.c
