[Bug c/116016] enhancement: add __builtin_set_counted_by(P->FAM, COUNT) or equivalent

[isanbard at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016

--- Comment #39 from Bill Wendling <isanbard at gmail dot com> ---
(In reply to qinzhao from comment #38)
> (In reply to Bill Wendling from comment #37)
> > That does make me wonder at the usefulness of this feature. The user will
> > need to set the count whether or not this builtin is used (either because
> > 'counted_by' wasn't specified or the compiler couldn't find the COUNT
> > variable for some reason (a compiler bug, but still...)). Isn't that
> > basically creating a feature useful for only this specific use case?
> 
> I think that this new builtin (actually a new C keyword) is useful to be
> provided to the user to help the programming experience when using the new
> attribute "counted_by", two basic use cases are described in comment #1, and
> #31. 
> 
> currently, there is a GCC patch being proposed recently
> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/658483.html
> 
> c: Add _Lengthof operator
> 
> From my understanding, the new __builtin_get_counted_by serves similar
> functionality as this potential future new C operator, but for flexiable
> array members or (later) pointers if we extend the counted-by to pointers.

But this all relies upon the 'counted_by' attribute existing. For this example:

  typeof(*__builtin_get_counted_by(P->FAM)) idx;

  for (idx = 0; idx < *__builtin_get_counted_by (P->FAM); idx++)
    do_things (P->FAM[idx]);

This needs an if-then around it:

  if (__builtin_has_attribute (P->FAM, counted_by)) {
    typeof(*__builtin_get_counted_by (P->FAM)) idx;

    for (idx = 0; idx < *__builtin_get_counted_by (P->FAM); idx++)
      do_things (P->FAM[idx]);
  }

Does this code need to be executed only when 'counted_by' exists? If so, it's
far better to write this, because the 'count' needs to be at the same "level"
as the FAM:

  if (__builtin_has_attribute (P->FAM, counted_by)) {
    typeof(P->COUNT) idx;

    for (idx = 0; idx < P->COUNT; idx++)
      do_things (P->FAM[idx]);
  }

If it must be executed whether or not 'counted_by' exists, then the first form
is overkill and requires duplication of code.

I know that it's just an example, but it shows the issue I'm having with this
feature request. The 'count' may be used in code that's not connected to
'counted_by'. In which case, the user will have to set the variable even after
the allocation:

  struct bar {
    ...
    int some_field;
    ...
  };

  struct foo {
    ...
    int counter;
    ...
    struct bar array[] __attribute__((counted_by(counter)));
  } *p;

  #define alloc(P, FAM, COUNT) ({ \
    typeof (P) __p;                                         \
    size_t __size = sizeof (*P) + sizeof (*P->FAM) * COUNT; \
    __p = kmalloc (__size, GFP);                            \
    __builtin_set_counted_by (__p->FAM, COUNT);             \
    __p;                                                    \
  })

  void allocate_foo (int count) {
    p = alloc (p, array, count);
    p->count = count; /* << ensure count is set for non-sanitizer uses */
  }

  void do_something (int val) {
    int idx;

    for (idx = 0; idx < p->count; ++idx)
      p->array[idx].some_field = val++;
  }

Perhaps I'm missing something...