[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

[Kees Cook via Phabricator via cfe-commits]

kees added inline comments.


================
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
----------------
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.. */
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126864/new/

https://reviews.llvm.org/D126864