Am Freitag, dem 26.01.2024 um 14:33 +0000 schrieb Qing Zhao:
>
> > 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?
I am not entirely sure but changing types in the FE seems
problematic because this breaks language semantics. And
then adding special code everywhere to treat it specially
in the FE does not seem a good way forward.
If I understand correctly, returning an incomplete array
type is not allowed and then fails during gimplification.
So I would suggest to make it return a pointer to the
incomplete array (and not the element type) but then wrap
it with an indirection when inserting this code in the FE
so that the full replacement has the correct type again
(of the incomplete array).
Alternatively, one could allow this during gimplification
or add some conversion.
Martin
>
> 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
> > >
> >
>
