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
