jyknight added a comment.
In D126864#3645994 <https://reviews.llvm.org/D126864#3645994>, @kees wrote:
> I should clarify: I still need the =3 mode. Since sizeof([0]) == 0 and
> sizeof([]) == error, they are being treated differently already by the
> compiler causing bugs in Linux. The kernel must still have a way to reject
> the _use_ of a [0] array. We cannot reject _declaration_ of them due to
> userspace API.
Looks like the linux kernel is currently chock-full of zero-length-arrays which
are actually intended/required to work as flexible array members. Do you have a
pending patch to convert all of them to [] which should be? If you're saying
you absolutely need this mode -- which I still believe would be nonsensical to
provide -- I'd like to see the set of concrete examples you cannot otherwise
deal with.
================
Comment at: clang/lib/CodeGen/CGExpr.cpp:906
// member, only a T[0] or T[] member gets that treatment.
+ // Under StrictFlexArraysLevel, obey c99+ that disallows FAM in union,
see
+ // C11 6.7.2.1 ยง18
----------------
kees wrote:
> jyknight wrote:
> > serge-sans-paille wrote:
> > > jyknight wrote:
> > > > I believe this bit is incorrect -- it should just go back to 'return
> > > > true;'. The StrictFlexArraysLevel check above already eliminates the
> > > > cases we want to eliminate (size==1 in strictness-level 2.)
> > > Well, if we are in strictness-level 2, with an undefined size or size =
> > > 0, we can still reach that path, and don't want to return 'true' because
> > > FAM in union are in invalid per the standard.
> > Yes, we can reach this path, which is why the change is incorrect. We
> > should not be changing the FAMness of undefined size, or size == 0, in any
> > of the modes. To be more specific --
> >
> > `union X { int x[0]; };` should still be a FAM in all strictness modes. (if
> > you don't want zero-length-arrays, use `-Werror=zero-length-array`).
> >
> > For `union X { int x[]; };`: this ought to be a compiler error. It's likely
> > just an oversight that we currently accept it; I'd be OK with a (separate)
> > patch to fix that. (GCC emits an error, so there's unlikely to be
> > compatibility issues with such a change.)
> > `union X { int x[0]; };` should still be a FAM in all strictness modes. (if
> > you don't want zero-length-arrays, use `-Werror=zero-length-array`).
>
> The Linux kernel cannot use `-Wzero-length-array` because we have cases of
> userspace APIs being stuck with them. (i.e. they are part of the struct
> declaration, even though the kernel code doesn't use them.) For example:
>
> ```
> In file included from ../kernel/bounds.c:13:
> In file included from ../include/linux/log2.h:12:
> In file included from ../include/linux/bitops.h:9:
> In file included from ../include/uapi/linux/kernel.h:5:
> ../include/uapi/linux/sysinfo.h:22:10: error: zero size arrays are an
> extension [-Werror,-Wzero-length-array]
> char _f[20-2*sizeof(__kernel_ulong_t)-sizeof(__u32)]; /* Padding:
> libc5 uses this.. */
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ```
>
ISTM you can simply remove this field on 64-bit platforms, without changing the
ABI. If you're worried about API breakage, for anything that might use the
field-name `_f` (?), you could make it visible only to user-space or
```
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wzero-length-array"
...
#pragma GCC diagnostic pop
```
if really needed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126864/new/
https://reviews.llvm.org/D126864
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
