> On Jan 26, 2024, at 3:04 AM, Martin Uecker <uec...@tugraz.at> wrote: > > > I haven't looked at the patch, but it sounds you give the result > the wrong type. Then patching up all use cases instead of the > type seems wrong.
Yes, this is for resolving a very early gimplification issue as I reported last Nov: https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638793.html Since no-one responded at that time, I fixed the issue by replacing the ARRAY_REF With a pointer indirection: https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639605.html The reason for such change is: return a flexible array member TYPE is not allowed by C language (our gimplification follows this rule), so, we have to return a pointer TYPE instead. ******The new internal function .ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, SIZE_OF_SIZE, ACCESS_MODE, INDEX) INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL) which returns the "REF_TO_OBJ" same as the 1st argument; Both the return type and the type of the first argument of this function have been converted from the incomplete array type to the corresponding pointer type. As a result, the original ARRAY_REF was converted to an INDIRECT_REF, the original INDEX of the ARRAY_REF was lost when converting from ARRAY_REF to INDIRECT_REF, in order to keep the INDEX for bound sanitizer instrumentation, I added The 6th argument “INDEX”. What’s your comment and suggestion on this solution? Thanks. Qing > > Martin > > > Am Donnerstag, dem 25.01.2024 um 20:11 +0000 schrieb Qing Zhao: >> Thanks a lot for the testing. >> >> Yes, I can repeat the issue with the following small example: >> >> #include <stdlib.h> >> #include <stddef.h> >> #include <stdio.h> >> >> #define MAX(a, b) ((a) > (b) ? (a) : (b)) >> >> struct untracked { >> int size; >> int array[] __attribute__((counted_by (size))); >> } *a; >> struct untracked * alloc_buf (int index) >> { >> struct untracked *p; >> p = (struct untracked *) malloc (MAX (sizeof (struct untracked), >> (offsetof (struct untracked, array[0]) >> + (index) * sizeof (int)))); >> p->size = index; >> return p; >> } >> >> int main() >> { >> a = alloc_buf(10); >> printf ("same_type is %d\n", >> (__builtin_types_compatible_p(typeof (a->array), typeof (&(a->array)[0])))); >> return 0; >> } >> >> >> /home/opc/Install/latest-d/bin/gcc -O2 btcp.c >> same_type is 1 >> >> Looks like that the “typeof” operator need to be handled specially in C FE >> for the new internal function .ACCESS_WITH_SIZE. >> >> (I have specially handle the operator “offsetof” in C FE already). >> >> Will fix this issue. >> >> Thanks. >> >> Qing >> >>> On Jan 24, 2024, at 7:51 PM, Kees Cook <keesc...@chromium.org> wrote: >>> >>> On Wed, Jan 24, 2024 at 12:29:51AM +0000, Qing Zhao wrote: >>>> This is the 4th version of the patch. >>> >>> Thanks very much for this! >>> >>> I tripped over an unexpected behavioral change that the Linux kernel >>> depends on: >>> >>> __builtin_types_compatible_p() no longer treats an array marked with >>> counted_by as different from that array's decayed pointer. Specifically, >>> the kernel uses these macros: >>> >>> >>> /* >>> * Force a compilation error if condition is true, but also produce a >>> * result (of value 0 and type int), so the expression can be used >>> * e.g. in a structure initializer (or where-ever else comma expressions >>> * aren't permitted). >>> */ >>> #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) >>> >>> #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) >>> >>> /* &a[0] degrades to a pointer: a different type from an array */ >>> #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) >>> >>> >>> This gets used in various places to make sure we're dealing with an >>> array for a macro: >>> >>> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + >>> __must_be_array(arr)) >>> >>> >>> So this builds: >>> >>> struct untracked { >>> int size; >>> int array[]; >>> } *a; >>> >>> __must_be_array(a->array) >>> => 0 (as expected) >>> __builtin_types_compatible_p(typeof(a->array), typeof(&(a->array)[0])) >>> => 0 (as expected, array vs decayed array pointer) >>> >>> >>> But if counted_by is added, we get a build failure: >>> >>> struct tracked { >>> int size; >>> int array[] __counted_by(size); >>> } *b; >>> >>> __must_be_array(b->array) >>> => build failure (not expected) >>> __builtin_types_compatible_p(typeof(b->array), typeof(&(b->array)[0])) >>> => 1 (not expected, both pointers?) >>> >>> >>> >>> >>> -- >>> Kees Cook >> >
