[PATCH] D133108: [clang] Rework IsTailPaddedMemberArray into isFlexibleArrayMemberExpr

serge via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 21 00:28:18 PDT 2022


serge-sans-paille added inline comments.


================
Comment at: clang/test/Sema/unbounded-array-bounds.c:101
+  char tail[1];  // addr16-note {{declared here}} addr32-note {{declared here}}
+} fam1;
+
----------------
msebor wrote:
> msebor wrote:
> > There's a difference between the sizes of `fam1` and `fam` that makes accesses to the four leading elements of `fam1.tail` strictly in bounds, while no access to either `fam.tail` or `fam0.tail` is (`sizeof fam` is the same as `sizeof int` while `sizeof fam1` is equal to `sizeof (int[2])` on common targets).  It would be helpful to capture that difference in the tests, both for the warning and for `__builtin_object_size`.
> > 
> > There should also be a difference between accessing elements of an object of an initialized struct with a flexible array member (i.e., one whose size is known) and those of an object that's only declared but that's defined in some other translation unit.  Since the size of the object is determined by its initializer, it should be reflected in `__builtin_object_size` and accesses to it checked by `-Warray-bounds`.  The size of the latter object is unknown it must be assumed to be `PTRDIFF_MAX - sizeof (int) - 1`.  It would also be helpful to add tests for these cases.
> > 
> > As far as I can see, none of these cases seems to be handled quite right on trunk.  For example, the size of `s` below should be 8 but Clang evaluates `__builtin_object_size(&s, N)` to 4, without diagnosing any past-the-end accesses to `s.a`:
> > ```
> > struct S {
> >   int n;
> >   char a[];
> > } s = { 1, { 2, 3, 4, 5 } };
> > ```
> I opened [[ https://github.com/llvm/llvm-project/issues/57860 | PR #57860 ]] to better show what I mean.
Agreed. I suggest we discuss that in this PR, while me merge this "code cleanup with enhancement", if @aaron.ballman agrees :-)


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

https://reviews.llvm.org/D133108



More information about the cfe-commits mailing list