Hi, Bill,
Thanks a lot for your testing case.
I studied this testing case and realized that we need to decide
what’s the expected behavior for the following situation:
struct bar;
struct a {
int dummy;
struct bar *array __attribute__((counted_by(count)));
char count;
};
when the size information of the element of the pointer array is not available
in the current compilation, i.e., there is no definition of the structure
“bar” in the
current file, the size of “structure bar” is not known, as a result,
compilation is not
able to compute the object size of the pointer array “array” even though the
length
of the array is known.
So, my question is:
1. When should the compiler issue warning for such situation?
A. During C frontend when checking the counted_by attributes.
B. During middle-end when __builtin_dynamic_object_size is computing the
object size.
I prefer B. The reason is: even though the counted_by attribute under such
situation is not enough for object_size,
It should be enough for the bound sanitizer?
Let me know your opinion on this.
thanks.
Qing
> On Jan 16, 2025, at 17:50, Bill Wendling <isanb...@gmail.com> wrote:
>
> I'm also getting this ICE:
>
> kiff ~/gcc $ cat g.c
> #include <stdio.h>
> #include <stdlib.h>
>
> #define __noinline __attribute__((noinline))
>
> struct bar;
>
> struct a {
> int dummy;
> struct bar *array __attribute__((counted_by(count)));
> char count;
> };
>
> struct a * __noinline alloc(int count) {
> struct a *ptr = malloc(sizeof(struct a) + sizeof(struct bar *) * count);
>
> ptr->count = count;
> return ptr;
> }
>
> int main() {
> struct a *ptr = alloc(10);
>
> printf("__bdos(ptr->array) == %lu\n",
> __builtin_dynamic_object_size(ptr->array, 0));
>
> return 0;
> }
>
>
> kiff ~/gcc $ ./gcc.install/bin/gcc -O2 g.c
> during GIMPLE pass: objsz
> g.c: In function ‘main’:
> g.c:21:5: internal compiler error: Segmentation fault
> 21 | int main() {
> | ^~~~
> 0x2645f5f internal_error(char const*, ...)
> ../../gcc.src/gcc/diagnostic-global-context.cc:517
> 0x1133e76 crash_signal
> ../../gcc.src/gcc/toplev.cc:322
> 0x7f82c195558f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0xd1d8b0 contains_struct_check(tree_node*, tree_node_structure_enum,
> char const*, int, char const*)
> ../../gcc.src/gcc/tree.h:3832
> 0xd1d8b0 fold_convert_loc(unsigned long, tree_node*, tree_node*)
> ../../gcc.src/gcc/fold-const.cc:2618
> 0x1201efe access_with_size_object_size
> ../../gcc.src/gcc/tree-object-size.cc:812
> 0x1201efe call_object_size
> ../../gcc.src/gcc/tree-object-size.cc:1429
> 0x1201efe collect_object_sizes_for
> ../../gcc.src/gcc/tree-object-size.cc:1883
> 0x12040ba merge_object_sizes
> ../../gcc.src/gcc/tree-object-size.cc:1470
> 0x12012f6 collect_object_sizes_for
> ../../gcc.src/gcc/tree-object-size.cc:1861
> 0x1202b7b compute_builtin_object_size(tree_node*, int, tree_node**)
> ../../gcc.src/gcc/tree-object-size.cc:1283
> 0xb6a5be fold_builtin_object_size
> ../../gcc.src/gcc/builtins.cc:11819
> 0xb6a5be fold_builtin_2
> ../../gcc.src/gcc/builtins.cc:10838
> 0xb6a5be fold_builtin_n
> ../../gcc.src/gcc/builtins.cc:10950
> 0x1203910 dynamic_object_sizes_execute_one
> ../../gcc.src/gcc/tree-object-size.cc:2187
> 0x1203910 object_sizes_execute
> ../../gcc.src/gcc/tree-object-size.cc:2250
> Please submit a full bug report, with preprocessed source (by using
> -freport-bug).
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
>
> On Thu, Jan 16, 2025 at 1:19 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>> Hi,
>>
>> This is the patch set to extend "counted_by" attribute to pointer fields of
>> structures.
>>
>> For example:
>>
>> struct PP {
>> size_t count2;
>> char other1;
>> char *array2 __attribute__ ((counted_by (count2)));
>> int other2;
>> } *pp;
>>
>> specifies that the "array2" is an array that is pointed by the
>> pointer field, and its number of elements is given by the field
>> "count2" in the same structure.
>>
>> Per the previous discussion with Martin and Bill
>> (https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669320.html)
>>
>> there are the following importand facts about "counted_by" on pointer fields
>> compared
>> to the "counted_by" on FAM fields:
>>
>> 1. one more new requirement for pointer fields with "counted_by" attribute:
>> pp->array2 and pp->count2 can ONLY be changed by changing the whole
>> structure
>> at the same time.
>>
>> 2. the following feature for FAM field with "counted_by" attribute is NOT
>> valid for the pointer field any more:
>>
>> " One important feature of the attribute is, a reference to the
>> flexible array member field uses the latest value assigned to the
>> field that represents the number of the elements before that
>> reference. For example,
>>
>> p->count = val1;
>> p->array[20] = 0; // ref1 to p->array
>> p->count = val2;
>> p->array[30] = 0; // ref2 to p->array
>>
>> in the above, 'ref1' uses 'val1' as the number of the elements in
>> 'p->array', and 'ref2' uses 'val2' as the number of elements in
>> 'p->array'. "
>>
>> Although in the previous discussion, I agreed with Martin that we should use
>> the
>> designator syntax (i.e, counted_by (.n) instead of counted_by (n)) for the
>> counted_by attribute for pointer fields, after more consideration and
>> discussion
>> with Bill Wendling (who is working on the same work for CLANG), we decided to
>> keep the current syntax of FAM for pointer fields. And leave the new syntax
>> (.n)
>> and more complicate expressions to a later work.
>>
>> This patch set includes 3 parts:
>>
>> 1.Extend "counted_by" attribute to pointer fields of structures.
>> 2.Convert a pointer reference with counted_by attribute to .ACCESS_WITH_SIZE
>> and use it in builtinin-object-size.
>> 3.Use the counted_by attribute of pointers in array bound checker.
>>
>> In which, the patch 1 and 2 are simple and straightforward, however, the
>> patch 3
>> is a little complicate due to the following reason:
>>
>> Current array bound checker only instruments ARRAY_REF, and the INDEX
>> information is the 2nd operand of the ARRAY_REF.
>>
>> When extending the array bound checker to pointer references with
>> counted_by attributes, the hardest part is to get the INDEX of the
>> corresponding array ref from the offset computation expression of
>> the pointer ref.
>>
>> The whole patch set has been bootstrapped and regression tested on both
>> aarch64
>> and x86.
>>
>> Let me know any comments and suggestions.
>>
>> Thanks.
>>
>> Qing
>>
>> Qing Zhao (3):
>> Extend "counted_by" attribute to pointer fields of structures.
>> Convert a pointer reference with counted_by attribute to
>> .ACCESS_WITH_SIZE and use it in builtinin-object-size.
>> Use the counted_by attribute of pointers in array bound checker.
>>
>> gcc/c-family/c-attribs.cc | 15 +-
>> gcc/c-family/c-gimplify.cc | 7 +
>> gcc/c-family/c-ubsan.cc | 264 ++++++++++++++++--
>> gcc/c/c-decl.cc | 91 +++---
>> gcc/c/c-typeck.cc | 41 +--
>> gcc/doc/extend.texi | 37 ++-
>> gcc/testsuite/gcc.dg/flex-array-counted-by.c | 2 +-
>> gcc/testsuite/gcc.dg/pointer-counted-by-2.c | 8 +
>> gcc/testsuite/gcc.dg/pointer-counted-by-3.c | 127 +++++++++
>> gcc/testsuite/gcc.dg/pointer-counted-by-4.c | 63 +++++
>> gcc/testsuite/gcc.dg/pointer-counted-by-5.c | 48 ++++
>> gcc/testsuite/gcc.dg/pointer-counted-by-6.c | 47 ++++
>> gcc/testsuite/gcc.dg/pointer-counted-by-7.c | 30 ++
>> gcc/testsuite/gcc.dg/pointer-counted-by-8.c | 30 ++
>> gcc/testsuite/gcc.dg/pointer-counted-by.c | 70 +++++
>> .../ubsan/pointer-counted-by-bounds-2.c | 47 ++++
>> .../ubsan/pointer-counted-by-bounds-3.c | 35 +++
>> .../ubsan/pointer-counted-by-bounds-4.c | 35 +++
>> .../ubsan/pointer-counted-by-bounds-5.c | 46 +++
>> .../ubsan/pointer-counted-by-bounds-6.c | 33 +++
>> .../gcc.dg/ubsan/pointer-counted-by-bounds.c | 46 +++
>> gcc/tree-object-size.cc | 11 +-
>> 22 files changed, 1045 insertions(+), 88 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-3.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-4.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-5.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-6.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-7.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by-8.c
>> create mode 100644 gcc/testsuite/gcc.dg/pointer-counted-by.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-3.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-4.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-5.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds-6.c
>> create mode 100644 gcc/testsuite/gcc.dg/ubsan/pointer-counted-by-bounds.c
>>
>> --
>> 2.31.1
>>
