Hi, Sid,
I further studied the approach to generate .ACCESS_WITH_SIZE in C FE for the
pointer with a STRUCTURE TYPE with counted-by FAM.
One thing bother me is the following:
For the following small example:
[ counted_by_whole]$ cat t.c
#include <stdlib.h>
#include <stddef.h>
struct annotated {
size_t count;
char other;
char array[] __attribute__((counted_by (count)));
};
#define MAX(A, B) (A > B) ? (A) : (B)
#define ALLOC_SIZE_ANNOTATED(COUNT) \
MAX(sizeof (struct annotated), \
offsetof(struct annotated, array[0]) + (COUNT) * sizeof(char))
static struct annotated * __attribute__((__noinline__)) alloc_buf (int index)
{
struct annotated *p;
p = (struct annotated *) malloc (ALLOC_SIZE_ANNOTATED(index));
p->count = index;
return p;
}
int main()
{
struct annotated *q = alloc_buf (10);
__builtin_printf ("the bdos whole is %d\n",
__builtin_dynamic_object_size(q, 1));
return 0;
}
The gimple IR is:
1 int main ()
2 {
3 int D.5072;
4
5 {
6 struct annotated * q;
7
8 q = alloc_buf (10);
9 _1 = __builtin_dynamic_object_size (q, 1);
10 __builtin_printf ("the bdos whole is %d\n", _1);
11 D.5072 = 0;
12 return D.5072;
13 }
14 D.5072 = 0;
15 return D.5072;
16 }
17
18
19 __attribute__((noinline))
20 struct annotated * alloc_buf (int index)
21 {
22 struct annotated * D.5074;
23 struct annotated * p;
24 25 _1 = (long unsigned int) index;
26 _2 = _1 + 9;
27 _3 = MAX_EXPR <_2, 16>;
28 p = malloc (_3);
29 _4 = (long unsigned int) index;
30 p->count = _4;
31 D.5074 = p;
32 return D.5074;
33 }
When we generate the .ACCESS_WITH_SIZE for a pointer reference to “struct
annotated”,
Looks like all the pointer references, at line 8, “q”, at line 9, “q”, at line
28, “p”, need to be changed
to a call to .ACCESS_WITH_SIZE. this might increase the IR size unnecessarily.
Might have some
Impact on the inlining decision heuristics or other heuristic that depend on
the code size.
Not sure whether this is a concern or not.
Any comment?
Qing
> On Apr 8, 2025, at 10:53, Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>
>> On Apr 7, 2025, at 22:15, Siddhesh Poyarekar <siddh...@gotplt.org> wrote:
>>
>> On 2025-04-07 14:53, Qing Zhao wrote:
>>>> Is there a reason to do this at the very end like this and not in the
>>>> GIMPLE_ASSIGN case in the switch block? Something like this:
>>>>
>>>> tree rhs = gimple_assign_rhs1 (stmt);
>>>> tree counted_by_ref = NULL_TREE;
>>>> if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR
>>>> || (gimple_assign_rhs_code (stmt) == ADDR_EXPR
>>>> && TREE_CODE (TREE_OPERAND (rhs, 0)) == MEM_REF))
>>>> reexamine = plus_stmt_object_size (osi, var, stmt);
>>>> else if (gimple_assign_rhs_code (stmt) == COND_EXPR)
>>>> reexamine = cond_expr_object_size (osi, var, stmt);
>>>> else if (gimple_assign_single_p (stmt)
>>>> || gimple_assign_unary_nop_p (stmt))
>>>> {
>>>> if (TREE_CODE (rhs) == SSA_NAME
>>>> && POINTER_TYPE_P (TREE_TYPE (rhs)))
>>>> reexamine = merge_object_sizes (osi, var, rhs);
>>>> else
>>>> expr_object_size (osi, var, rhs);
>>>> }
>>>> + else if ((counted_by_ref = fam_struct_with_counted_by (rhs)))
>>>> + record_fam_object_size (osi, var, counted_by_ref);
>>>> else
>>>> unknown_object_size (osi, var);
>>>>
>>>> where you can then fold in all your gating conditions, including getting
>>>> the counted_by ref into the fam_struct_with_counted_by and then limit the
>>>> record_fam_object_size to just evaluating the type size + counted_by size.
>>>>
>>>> This may even help avoid the insertion order hack you have to do, i.e. the
>>>> gsi_insert_seq_before vs gsi_insert_seq_after.
>>> This is a good suggestion. I will try this to see any issue there.
>>> My initial thought is to give the counted_by information the lowest priority
>>> if there are other information (for example, malloc) available.
>>> Do you see any issue here?
>>
>> No, that is the right idea, but I don't know if you'll actually need to
>> choose. AFAICT, you'll either be able to trace the pointer to an
>> allocation, in which case the fallback is unnecessary. Otherwise you'll
>> trace it to one of the following:
>>
>> 1. An assignment from an expression that returns the pointer
>> 2. A NOP with a var_decl, which is handled in the GIMPLE_NOP case; you'd
>> need to add a similar hook there.
>>
>> I can't think of any other cases off the top of my head, how about you?
>>
>>>> Also, it seems like simply making build_counted_by_ref available may be
>>>> unnecessary and maybe you could explore associating the counted_by
>>>> component_ref with the parent type somehow. Alternatively, how about
>>>> building an .ACCESS_WITH_SIZE for types with FAM that have a counted_by?
>>>> That would allow the current access_with_size_object_size() to work out of
>>>> the box.
>>> I am not sure about this though.
>>> Our initial design is to change every component_ref (which is a reference
>>> to the FAM)
>>> in the data flow of the routine that has a counted_by attribute to a call
>>> to .ACCESS_WITH_SIZE.
>>> Then put this call to .ACCESS_WITH_SIZE into the data flow of the routine.
>>> Now, if we try to associate counted_by information to the parent type, how
>>> can we add such information
>>> To the data flow of the routine if there is no explicit reference to the
>>> array itself?
>>
>> I'm not entirely sure, but maybe whenever there is an access on a ptr to the
>> parent struct, add a call to .ACCESS_WITH_SIZE there, with a built
>> expression for its size? e.g for:
>>
>> struct S
>> {
>> size_t c;
>> char a[] __counted_by (c);
>> }
>>
>> void foo (Struct S *s)
>> {
>> ...
>> sz = __builtin_dynamic_object_size (s, 0);
>> ...
>> }
>>
>> we could generate:
>>
>> void foo (struct S *s)
>> {
>> ...
>> sz_exp = c + sizeof (struct S);
>> s_1 = .ACCESS_WITH_SIZE (&s..., &c, ...);
>> ...
>> sz = __builtin_dynamic_object_size (*s_1, 0);
>> }
>>
>
> Yes, I prefer this approach.
>
> For the definition of .ACCESS_WITH_SIZE:
>
> ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
> TYPE_OF_SIZE, ACCESS_MODE)
> which returns the REF_TO_OBJ same as the 1st argument;
>
> 1st argument REF_TO_OBJ: The reference to the object;
> 2nd argument REF_TO_SIZE: The reference to the size of the object,
> 3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE
> represents
> 0: the number of bytes.
> 1: the number of the elements of the object type;
> 4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the
> TYPE
> of the object referenced by REF_TO_SIZE
> 5th argument ACCESS_MODE:
> -1: Unknown access semantics
> 0: none
> 1: read_only
> 2: write_only
> 3: read_write
> 6th argument: A constant 0 with the pointer TYPE to the original flexible
> array type.
>
> Currently, we only generate .ACCESS_WITH_SIZE for p->array if it’s a FAM with
> counted_by, the 3rd argument
> of this call is 1 (the number of the elements of the object type). And this
> information can be used in both __builtin_object_size
> and array bound sanitizer.
>
> For a reference to a structure with FAM, such as p, we can generate a call to
> .ACCESS_WITH_SIZE whose 3rd argument
> is 0 (the number of bytes). And this information can be used in
> __builtin_object_size. But not in array bound sanitizer.
>
> So, for your above example:
>
> struct S
> {
> size_t c;
> char a[] __counted_by (c);
> }
>
> void foo (Struct S *s)
> {
> ...
> sz = __builtin_dynamic_object_size (s, 0);
> …
> }
>
> C FE could generate:
>
> void foo (struct S *s)
> {
> …
> If (!type_unsigned (typeof (s->c))
> s->c = (s->c < 0) ? 0 : s->c;
> sz_exp = MAX (sizeof (struct S), offset (struct S, a) + s->c * sizeof (char));
> s_1 = .ACCESS_WITH_SIZE (&s, &sz_exp, 0, ...);
> ...
> sz = __builtin_dynamic_object_size (*s_1, 0);
> }
>
>
> Then, in the tree-object-size.cc <http://tree-object-size.cc/>, we can use
> the path to access_with_size_object_size() directly.
>
> How do you think?
>
> Thanks a lot for your suggestion.
>
> Qing
>> or something like that. But like I said, it's an alternative idea to avoid
>> special-casing in tree-object-size, which should provide size information
>> across all passes not just for object size.
>>
>> Thanks,
>> Sid
