Hi,
Thanks for the information.
Yes, providing a unary operator similar as __counted_by(PTR) as suggested by
multiple people previously is a cleaner approach.
Then the programmer will use the following:
__builtin_choose_expr(
__builtin_has_attribute (__p->FAM, "counted_by”)
__builtin_get_counted_by(__p->FAM) = COUNT, 0);
From the programmer’s point of view, it’s cleaner too.
However, there is one issue with “__builtin_choose_expr” currently in GCC, its
documentation explicitly mentions this limitation:
(https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fchoose_005fexpr)
"Note: This construct is only available for C. Furthermore, the unused
expression (exp1 or exp2 depending on the value of const_exp) may still
generate syntax errors. This may change in future revisions.”
So, due to this limitation, when there is no counted_by attribute, the
__builtin_get_counted_by() still is evaluated by the compiler and errors is
issued and the compilation stops, this can be show from the small testing case:
[opc@qinzhao-ol8u3-x86 gcc]$ cat ttt.c
struct flex {
unsigned int b;
int c[];
} *array_flex;
#define MY_ALLOC(P, FAM, COUNT) ({ \
typeof(P) __p; \
unsigned int __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
__p = (typeof(P)) __builtin_malloc(__size); \
__builtin_choose_expr( \
__builtin_has_attribute (__p->FAM, counted_by), \
__builtin_counted_by_ref(__p->FAM) = COUNT, 0); \
P = __p; \
})
int main(int argc, char *argv[])
{
MY_ALLOC(array_flex, c, 20);
return 0;
}
[opc@qinzhao-ol8u3-x86 gcc]$ sh t
ttt.c: In function ‘main’:
ttt.c:13:5: error: the argument must have ‘counted_by’ attribute
‘__builtin_counted_by_ref’
ttt.c:19:3: note: in expansion of macro ‘MY_ALLOC’
I checked the FE code on handling “__buiiltin_choose_expr”, Yes, it does parse
the __builtin_counted_by_ref(__p->FAM) even when
__builtin_has_attribute(__p->FAM, counted_by) is FALSE, and issued the error
when parsing __builtin_counted_by_ref and stopped the compilation.
So, in order to support this approach, we first must fix the issue in the
current __builtin_choose_expr in GCC. Otherwise, it’s impossible for the user
to use this new builtin.
Let me know your comments and suggestions.
thanks.
Qing
> On Aug 27, 2024, at 14:46, Bill Wendling <[email protected]> wrote:
>>>
>>> It would make it easier for your use case. I wonder though
>>> whether other people might want to have the compile time error
>>> when there is no attribute.
>>
>> Then this will go back to the previous discussion: whether we should go the
>> approach for the unary operator __counted_by(PTR), then the other builtin
>> __builtin_has_attribute(PTR, counted_by) should be provided to the user.
>>
>> For GCC, there is no issue, we already have the __builtin_has_attribute
>> (PTR, counted_by) available.
>> But CLANG doesn’t have this builtin currently, need to implement it before
>> the unary operator __counted_by(PTR).
>>
>> Let me know your opinion.
>
> We have a bit of an issue with the current design. The group at Apple,
> who are implementing a lot of the bounds safety checks need to prevent
> the address of the 'count' from being taken. See [1] for details, but
> the upshot is that they try to ensure that the 'count' and flexible
> array member aren't modified independently. The discussion is ongoing,
> but they suggested an alternative that is similar to ones discussed in
> this thread and the bugzilla.
>
> A brief description of their proposal is adding the builtin
> '__builtin_bounds_attr_arg'. It would take counted_by's argument and
> return the counter directly:
>
> __builtin_choose_expr(
> __builtin_has_attribute(__p->FAM, "counted_by"),
> __builtin_bounds_attr_arg(__p->FAM) = COUNT,
> (void)
> );
>
> This is similar to one of the original proposals for
> __builtin_get_counted_by, but we changed because Clang didn't have
> __builtin_has_attribute. Clang should implement
> __builtin_has_attribute regardless, and if this is a better way of
> doing things, and doesn't interfere with Apple's work, then I can add
> __builtin_has_attribute to Clang so that we "do the right thing."
>
> As I said though, the discussion is ongoing at [1], but just wanted to
> give everyone a warning, and feel free to jump in with comments /
> suggestions.
>
> [1]
> https://discourse.llvm.org/t/rfc-introducing-new-clang-builtin-builtin-get-counted-by/80836
>
> -bw
