Am Freitag, dem 06.09.2024 um 13:59 +0000 schrieb Qing Zhao:
>
> > On Sep 5, 2024, at 18:22, Bill Wendling <isanb...@gmail.com> wrote:
> >
> > Hi Qing,
> >
> > Sorry for my late reply.
> >
> > On Thu, Aug 29, 2024 at 7:22 AM Qing Zhao <qing.z...@oracle.com> wrote:
> > >
> > > 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.
> > >
> > Do you need to emit a diagnostic if the FAM doesn't have the
> > counted_by attribute? It was originally supposed to "silently fail" if
> > it didn't. We may need to do the same for Clang if so.
>
> Yes, “silently fail” should workaround this problem if fixing the issue in
> the current __builtin_choose_expr is too complicate.
>
> I will study a little bit on how to fix the issue in __builtin_choose_expr
> first.
>
> Martin and Joseph, any comment or suggestion from you?
My recommendation would be not to change __builtin_choose_expr.
The design where __builtin_get_counted_by returns a null
pointer constant (void*)0 seems good. Most users will
get an error which I think is what we want and for those
that want it to work even if the attribute is not there, the
following code seems perfectly acceptable to me:
auto p = __builtin_get_counted_by(__p->FAM)
*_Generic(p, void*: &(int){}, default: p) = 1;
Kees also seemed happy with it. And if I understood it correctly,
also Clang's bounds checking people can work with this.
Martin
