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.

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
> 

Reply via email to