void marked an inline comment as done.
void added inline comments.
================
Comment at: clang/test/CodeGen/bounds-checking-fam.c:2
// REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=0
-fsanitize=array-bounds %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-0
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=0
-fsanitize=array-bounds -x c++ %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=1
-fsanitize=array-bounds %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-1
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=1
-fsanitize=array-bounds -x c++ %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-1,CXX
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=2
-fsanitize=array-bounds %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-2
-// RUN: %clang_cc1 -emit-llvm -triple x86_64 -fstrict-flex-arrays=2
-fsanitize=array-bounds -x c++ %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-2,CXX
+// RUN: %clang_cc1 -O2 -emit-llvm -triple x86_64 -fstrict-flex-arrays=0
-fsanitize=array-bounds %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-0
+// RUN: %clang_cc1 -O2 -emit-llvm -triple x86_64 -fstrict-flex-arrays=0
-fsanitize=array-bounds -x c++ %s -o - | FileCheck %s
--check-prefixes=CHECK,CHECK-STRICT-0,CXX,CXX-STRICT-0
----------------
serge-sans-paille wrote:
> Why adding -02 here?
Mostly because that's how Linux compiles its files. It also mirrors the test
below. I'm not super married to it if you'd rather I omit it.
================
Comment at: clang/test/CodeGen/object-size-flex-array.c:45
+ // CHECK-STRICT-2: ret i32 -1
+ // CHECK-STRICT-3: ret i32 0
return OBJECT_SIZE_BUILTIN(f->c, 1);
----------------
serge-sans-paille wrote:
> kees wrote:
> > serge-sans-paille wrote:
> > > This one worries me a bit, as an array of size 0 is invalid C, unless you
> > > consider the extension of it being a FAM. Shouldn't we emit a warning or
> > > something here? @kees what meaning would you give to that construct under
> > > `-fstrict-flex-arrays=3` ?
> > ```
> > type identifier[0]
> > ```
> > when not a fake FAM is the same as:
> >
> > ```
> > struct { } identifier
> > ```
> >
> > It's addressable with no size.
> >
> > FWIW, this is how GCC is treating it, and opted for no warning. The warning
> > gains nothing and is likely an irritant: if someone is requesting =3, they
> > want this behavior. If they didn't, they'd just use `-Wzero-length-array`.
> > In particular, the Linux kernel is in the position of needing to have
> > zero-length arrays (legacy userspace API) alongside real FAMs, even if we
> > don't reference the zero-length members. So we cannot use
> > '-Wzero-length-array', but we want to make sure no zero-length arrays will
> > ever be used as fake FAMs, as a code quality/style enforcement. This is
> > especially true of FORTIFY_SOURCE, where `__bos()` needs to report 0
> > instead of -1 for such a destination buffer size, so that we immediately
> > trip compile-time (or at worst, run-time) warnings, to keep any kernel
> > internals from using the deprecated members as a fake FAM.
> >
> > Take this case:
> >
> > ```
> > struct broken {
> > int foo;
> > int fake_fam[0];
> > struct something oops;
> > };
> > ```
> >
> > There have been bugs where the above struct was created because "oops" got
> > added after "fake_fam" by someone not realizing. Under FORTIFY_SOURCE,
> > doing:
> >
> > ```
> > memcpy(p->fake_fam, src, len);
> > ```
> >
> > raises no warning when `__bos(p->fake_fam, 1)` returns -1 and will happily
> > stomp on "oops". If `__bos()` returns 0, we can compile-time (or run-time)
> > block the memcpy. (And this holds for -fsanitize=bounds as well: if it is
> > considered to be unknown size, it won't trip on access, but if it's
> > 0-sized, it'll trip.)
> >
> > So, we can't keep zero-length arrays out of the kernel, but we want to be
> > able to enforce that if they DO show up, they will trip warnings quickly.
> >
> Thanks for clarifying the situation, esp. the kernel background. This looks
> like a quite specific scenario, but it's fine with me.
I'll add @kees's comment to the commit message.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134902/new/
https://reviews.llvm.org/D134902
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
