Okay. This previous small example was used to show the correct behavior of
__bos
for Fixed arrays when the allocation size and the TYPE_SIZE are mismatched.
Now we agreed on the correct behavior for each of the cases for the fixed array.
Since the new “counted_by” attribute is mainly a complement to the TYPE_SIZE
for the flexible array member.
So, GCC should just use it similarly as TYPE_SIZE.
Based on the fixed array example, I came up a small example for the flexible
array member with “counted_by” attribute,
And the expected correct behavior for each of the cases.
I also put detailed comments into the example to explain why for each case.
(Similar as the fixed array example)
Please take a look at this example and let me know any issue you see.
With my private GCC that support “counted_by” attribute, all the cases passed.
Thanks.
Qing.
========================================================
#include <stdint.h>
#include <malloc.h>
struct annotated {
size_t foo;
int array[] __attribute__((counted_by (foo)));
};
#define expect(p, _v) do { \
size_t v = _v; \
if (p == v) \
__builtin_printf ("ok: %s == %zd\n", #p, p); \
else \
{ \
__builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
} \
} while (0);
#define noinline __attribute__((__noinline__))
#define SIZE_BUMP 5
/* In general, Due to type casting, the type for the pointee of a pointer
does not say anything about the object it points to,
So, __builtin_object_size can not directly use the type of the pointee
to decide the size of the object the pointer points to.
there are only two reliable ways:
A. observed allocations (call to the allocation functions in the routine)
B. observed accesses (read or write access to the location of the
pointer points to)
that provide information about the type/existence of an object at
the corresponding address.
for A, we use the "alloc_size" attribute for the corresponding allocation
functions to determine the object size;
For B, we use the SIZE info of the TYPE attached to the corresponding access.
(We treat counted_by attribute as a complement to the SIZE info of the TYPE
for FMA)
The only other way in C which ensures that a pointer actually points
to an object of the correct type is 'static':
void foo(struct P *p[static 1]);
See https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624814.html
for more details. */
/* in the following function, malloc allocated more space than the value
of counted_by attribute. Then what's the correct behavior we expect
the __builtin_dynamic_object_size should have for each of the cases? */
static struct annotated * noinline alloc_buf_more (int index)
{
struct annotated *p;
p = malloc(sizeof (*p) + (index + SIZE_BUMP) * sizeof (int));
p->foo = index;
/*when checking the observed access p->array, we have info on both
observered allocation and observed access,
A. from observed allocation: (index + SIZE_BUMP) * sizeof (int)
B. from observed access: p->foo * sizeof (int)
in the above, p->foo = index.
*/
/* for size in the whole object: always uses A. */
/* for size in the sub-object: chose the smaller of A and B.
* Please see https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625891.html
* for details on why. */
/* for MAXIMUM size in the whole object: use the allocation size
for the whole object. */
expect(__builtin_dynamic_object_size(p->array, 0), (index + SIZE_BUMP) *
sizeof(int));
/* for MAXIMUM size in the sub-object. use the smaller of A and B. */
expect(__builtin_dynamic_object_size(p->array, 1), (p->foo) * sizeof(int));
/* for MINIMUM size in the whole object: use the allocation size
for the whole object. */
expect(__builtin_dynamic_object_size(p->array, 2), (index + SIZE_BUMP) *
sizeof(int));
/* for MINIMUM size in the sub-object: use the smaller of A and B. */
expect(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(int));
/*when checking the pointer p, we only have info on the observed allocation.
So, the object size info can only been obtained from the call to malloc.
for both MAXIMUM and MINIMUM: A = (index + SIZE_BUMP) * sizeof (int) */
expect(__builtin_dynamic_object_size(p, 1), sizeof (*p) + (index + SIZE_BUMP)
* sizeof(int));
expect(__builtin_dynamic_object_size(p, 0), sizeof (*p) + (index + SIZE_BUMP)
* sizeof(int));
expect(__builtin_dynamic_object_size(p, 3), sizeof (*p) + (index + SIZE_BUMP)
* sizeof(int));
expect(__builtin_dynamic_object_size(p, 2), sizeof (*p) + (index + SIZE_BUMP)
* sizeof(int));
return p;
}
/* in the following function, malloc allocated less space than the value
of counted_by attribute. Then what's the correct behavior we expect
the __builtin_dynamic_object_size should have for each of the cases?
NOTE: this is an user error, GCC should issue warnings for such case.
this is a seperate issue we should address later. */
static struct annotated * noinline alloc_buf_less (int index)
{
struct annotated *p;
p = malloc(sizeof (*p) + (index) * sizeof (int));
p->foo = index + SIZE_BUMP;
/*when checking the observed access p->array, we have info on both
observered allocation and observed access,
A. from observed allocation: (index) * sizeof (int)
B. from observed access: p->foo * sizeof (int)
in the above, p->foo = index + SIZE_BUMP.
*/
/* for size in the whole object: always uses A. */
/* for size in the sub-object: chose the smaller of A and B.
* Please see https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625891.html
* for details on why. */
/* for MAXIMUM size in the whole object: use the allocation size
for the whole object. */
expect(__builtin_dynamic_object_size(p->array, 0), (index) * sizeof(int));
/* for MAXIMUM size in the sub-object. use the smaller of A and B. */
expect(__builtin_dynamic_object_size(p->array, 1), (index) * sizeof(int));
/* for MINIMUM size in the whole object: use the allocation size
for the whole object. */
expect(__builtin_dynamic_object_size(p->array, 2), (index) * sizeof(int));
/* for MINIMUM size in the sub-object: use the smaller of A and B. */
expect(__builtin_dynamic_object_size(p->array, 3), (index) * sizeof(int));
/*when checking the pointer p, we only have info on the observed allocation.
So, the object size info can only been obtained from the call to malloc.
for both MAXIMUM and MINIMUM: A = (index + SIZE_BUMP) * sizeof (int) */
expect(__builtin_dynamic_object_size(p, 1), sizeof (*p) + (index) *
sizeof(int));
expect(__builtin_dynamic_object_size(p, 0), sizeof (*p) + (index) *
sizeof(int));
expect(__builtin_dynamic_object_size(p, 3), sizeof (*p) + (index) *
sizeof(int));
expect(__builtin_dynamic_object_size(p, 2), sizeof (*p) + (index) *
sizeof(int));
return p;
}
int main ()
{
struct annotated *p, *q;
p = alloc_buf_more (10);
q = alloc_buf_less (10);
/*when checking the observed access p->array, we only have info on the
observed access, i.e, the TYPE_SIZE info from the access. We don't have
info on the whole object. */
expect(__builtin_dynamic_object_size(p->array, 1), p->foo * sizeof(int));
expect(__builtin_dynamic_object_size(p->array, 0), -1);
expect(__builtin_dynamic_object_size(p->array, 3), p->foo * sizeof(int));
expect(__builtin_dynamic_object_size(p->array, 2), 0);
/*when checking the pointer p, we have no observed allocation nor observed
access.
therefore, we cannot determine the size info here. */
expect(__builtin_dynamic_object_size(p, 1), -1);
expect(__builtin_dynamic_object_size(p, 0), -1);
expect(__builtin_dynamic_object_size(p, 3), 0);
expect(__builtin_dynamic_object_size(p, 2), 0);
/*when checking the observed access p->array, we only have info on the
observed access, i.e, the TYPE_SIZE info from the access. We don't have
info on the whole object. */
expect(__builtin_dynamic_object_size(q->array, 1), q->foo * sizeof(int));
expect(__builtin_dynamic_object_size(q->array, 0), -1);
expect(__builtin_dynamic_object_size(q->array, 3), q->foo * sizeof(int));
expect(__builtin_dynamic_object_size(q->array, 2), 0);
/*when checking the pointer p, we have no observed allocation nor observed
access.
therefore, we cannot determine the size info here. */
expect(__builtin_dynamic_object_size(q, 1), -1);
expect(__builtin_dynamic_object_size(q, 0), -1);
expect(__builtin_dynamic_object_size(q, 3), 0);
expect(__builtin_dynamic_object_size(q, 2), 0);
return 0;
}
/home/opc/Install/latest-d/bin/gcc -O -fstrict-flex-arrays=3 t_final.c
ok: __builtin_dynamic_object_size(p->array, 0) == 60
ok: __builtin_dynamic_object_size(p->array, 1) == 40
ok: __builtin_dynamic_object_size(p->array, 2) == 60
ok: __builtin_dynamic_object_size(p->array, 3) == 40
ok: __builtin_dynamic_object_size(p, 1) == 68
ok: __builtin_dynamic_object_size(p, 0) == 68
ok: __builtin_dynamic_object_size(p, 3) == 68
ok: __builtin_dynamic_object_size(p, 2) == 68
ok: __builtin_dynamic_object_size(p->array, 0) == 40
ok: __builtin_dynamic_object_size(p->array, 1) == 40
ok: __builtin_dynamic_object_size(p->array, 2) == 40
ok: __builtin_dynamic_object_size(p->array, 3) == 40
ok: __builtin_dynamic_object_size(p, 1) == 48
ok: __builtin_dynamic_object_size(p, 0) == 48
ok: __builtin_dynamic_object_size(p, 3) == 48
ok: __builtin_dynamic_object_size(p, 2) == 48
ok: __builtin_dynamic_object_size(p->array, 1) == 40
ok: __builtin_dynamic_object_size(p->array, 0) == -1
ok: __builtin_dynamic_object_size(p->array, 3) == 40
ok: __builtin_dynamic_object_size(p->array, 2) == 0
ok: __builtin_dynamic_object_size(p, 1) == -1
ok: __builtin_dynamic_object_size(p, 0) == -1
ok: __builtin_dynamic_object_size(p, 3) == 0
ok: __builtin_dynamic_object_size(p, 2) == 0
ok: __builtin_dynamic_object_size(q->array, 1) == 60
ok: __builtin_dynamic_object_size(q->array, 0) == -1
ok: __builtin_dynamic_object_size(q->array, 3) == 60
ok: __builtin_dynamic_object_size(q->array, 2) == 0
ok: __builtin_dynamic_object_size(q, 1) == -1
ok: __builtin_dynamic_object_size(q, 0) == -1
ok: __builtin_dynamic_object_size(q, 3) == 0
ok: __builtin_dynamic_object_size(q, 2) == 0
[opc@qinzhao-ol8u3-x86 108896]$
> On Aug 1, 2023, at 6:57 PM, Kees Cook <keesc...@chromium.org> wrote:
>
> On Tue, Aug 01, 2023 at 09:35:30PM +0000, Qing Zhao wrote:
>>
>>
>>> On Jul 31, 2023, at 1:07 PM, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
>>>
>>> On 2023-07-31 13:03, Siddhesh Poyarekar wrote:
>>>> On 2023-07-31 12:47, Qing Zhao wrote:
>>>>> Hi, Sid and Jakub,
>>>>>
>>>>> I have a question in the following source portion of the routine
>>>>> “addr_object_size” of gcc/tree-object-size.cc:
>>>>>
>>>>> 743 bytes = compute_object_offset (TREE_OPERAND (ptr, 0), var);
>>>>> 744 if (bytes != error_mark_node)
>>>>> 745 {
>>>>> 746 bytes = size_for_offset (var_size, bytes);
>>>>> 747 if (var != pt_var && pt_var_size && TREE_CODE (pt_var) ==
>>>>> MEM_REF)
>>>>> 748 {
>>>>> 749 tree bytes2 = compute_object_offset (TREE_OPERAND
>>>>> (ptr, 0),
>>>>> 750 pt_var);
>>>>> 751 if (bytes2 != error_mark_node)
>>>>> 752 {
>>>>> 753 bytes2 = size_for_offset (pt_var_size, bytes2);
>>>>> 754 bytes = size_binop (MIN_EXPR, bytes, bytes2);
>>>>> 755 }
>>>>> 756 }
>>>>> 757 }
>>>>>
>>>>> At line 754, why we always use “MIN_EXPR” whenever it’s for OST_MINIMUM
>>>>> or not?
>>>>> Shall we use
>>>>>
>>>>> (object_size_type & OST_MINIMUM
>>>>> ? MIN_EXPR : MAX_EXPR)
>>>>>
>>>> That MIN_EXPR is not for OST_MINIMUM. It is to cater for allocations like
>>>> this:
>>>> typedef struct
>>>> {
>>>> int a;
>>>> } A;
>>>> size_t f()
>>>> {
>>>> A *p = malloc (1);
>>>> return __builtin_object_size (p, 0);
>>>
>>> Correction, that should be __builtin_object_size (p->a, 0).
>>
>> Actually, it should be __builtin_object_size(p->a, 1).
>> For __builtin_object_size(p->a,0), gcc always uses the allocation size for
>> the whole object.
>>
>> GCC’s current behavior is:
>>
>> For the size of the whole object, GCC currently always uses the allocation
>> size.
>> And for the size in the sub-object, GCC chose the smaller one among the
>> allocation size and the TYPE_SIZE.
>>
>> Is this correct behavior?
>>
>> thanks.
>>
>> Qing
>>
>> Please see the following small example to show the above behavior:
>>
>> =====
>>
>> #include <stdint.h>
>> #include <malloc.h>
>>
>> #define LENGTH 10
>> #define SIZE_BUMP 5
>> #define noinline __attribute__((__noinline__))
>>
>> struct fix {
>> size_t foo;
>> int array[LENGTH];
>> };
>>
>> #define expect(p, _v) do { \
>> size_t v = _v; \
>> if (p == v) \
>> __builtin_printf ("ok: %s == %zd\n", #p, p); \
>> else \
>> { \
>> __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
>> } \
>> } while (0);
>>
>>
>> /* in the following function, malloc allocated more space than size of the
>> struct fix. Then what's the correct behavior we expect
>> the __builtin_object_size should have for the following?
>> */
>>
>> static struct fix * noinline alloc_buf_more ()
>> {
>> struct fix *p;
>> p = malloc(sizeof (struct fix) + SIZE_BUMP * sizeof (int));
>>
>> /*when checking the observed access p->array, we have info on both
>> observered allocation and observed access,
>> A. from observed allocation (alloc_size): (LENGTH + SIZE_BUMP) * sizeof
>> (int)
>> B. from observed access (TYPE): LENGTH * sizeof (int)
>> */
>>
>> /* for MAXIMUM size in the whole object: currently, GCC always used the A.
>> */
>> expect(__builtin_object_size(p->array, 0), (LENGTH + SIZE_BUMP) *
>> sizeof(int));
>
> ok: __builtin_object_size(p->array, 0) == 60
>
> This is what I'd expect, yes: all memory from "array" to end of
> allocation, and that matches here: (LENGTH + SIZE_BUMP) * sizeof(int)
>
>>
>> /* for MAXIMUM size in the sub-object: currently, GCC chose the smaller
>> one among these two: B. */
>> expect(__builtin_object_size(p->array, 1), LENGTH * sizeof(int));
>
> ok: __builtin_object_size(p->array, 1) == 40
>
> Also as I'd expect: just LENGTH * sizeof(int), the remaining bytes
> starting at "array", based on type info, regardless of rest of allocation.
>
>>
>> return p;
>> }
>>
>> /* in the following function, malloc allocated less space than size of the
>> struct fix. Then what's the correct behavior we expect
>> the __builtin_object_size should have for the following?
>> */
>>
>> static struct fix * noinline alloc_buf_less ()
>> {
>> struct fix *p;
>> p = malloc(sizeof (struct fix) - SIZE_BUMP * sizeof (int));
>>
>> /*when checking the observed access p->array, we have info on both
>> observered allocation and observed access,
>> A. from observed allocation (alloc_size): (LENGTH - SIZE_BUMP) * sizeof
>> (int)
>> B. from observed access (TYPE): LENGTH * sizeof (int)
>> */
>>
>> /* for MAXIMUM size in the whole object: currently, GCC always used the A.
>> */
>> expect(__builtin_object_size(p->array, 0), (LENGTH - SIZE_BUMP) *
>> sizeof(int));
>
> ok: __builtin_object_size(p->array, 0) == 20
>
> My brain just melted a little, as this is now an under-sized instance of
> "p", so we have an incomplete allocation. (I would expect -Warray-bounds
> to yell very loudly for this.) But, technically, yes, this looks like
> the right calculation.
>
>>
>> /* for MAXIMUM size in the sub-object: currently, GCC chose the smaller
>> one among these two: B. */
>> expect(__builtin_object_size(p->array, 1), (LENGTH - SIZE_BUMP) *
>> sizeof(int));
>
> ok: __builtin_object_size(p->array, 1) == 20
>
> Given the under-allocation, yes, this seems correct to me, though,
> again, I would expect a compile-time warning. (Or at least, I've seen
> -Warray-bounds yell about this kind of thing in the past.)
>
> -Kees
>
> --
> Kees Cook
