kees added a comment.

In D135727#3853896 <https://reviews.llvm.org/D135727#3853896>, @void wrote:

> @kees @serge-sans-paille: It appears to me that a terminating array of size > 
> 2 *isn't* treated as a FAM in Clang, at least Clang warns about it. The first 
> failure above (`clang/test/Sema/array-bounds-ptr-arith.c`) shows that. It 
> turns out that the same failure occurs in that testcase when the array isn't 
> the last in a structure, so I'll change it.

Okay, what the heck is even going on in this test? The diagnostic appears to 
think the array changes in size based on the cast??

  <source>:16:24: warning: the pointer incremented by 80 refers past the end of 
the array (that contains 64 elements) [-Warray-bounds-pointer-arithmetic]
          return (void *)es->s_uuid + 80;
                         ^            ~~

s_uuid is 8, not 64. 64 would be 8 "void *"s.  This seems like a very very 
broken diagnostic?

These should all trip the diagnostic, but don't:

  es->s_uuid + 8; /* this is past the end */
  (void *)es->s_uuid + 9; /* this is past the end by 1, but doesn't trip 
because it thinks it's suddenly 8 times larger */

For -Warray-bounds, GCC isn't fooled by the "void *" casts (but doesn't have 
-Warray-bounds-arithmetic):
https://godbolt.org/z/5eqEEv4of

Note that while the diagnostics of both GCC and Clang complain only about >1 
terminal arrays, they both return -1 for `__builtin_object_size` regardless of 
length.

So, we're facing, again, a disconnect between diagnostics, bos, and sanitizer. 
GCC's sanitizer follows bos rules, where-as Clang's sanitizer followed 
diagnostics rules. Given that bos is used for run-time analysis, it follows 
that sanitizer and bos should match.

> There's another failure that I'm not too sure about. The 
> `Clang.SemaCXX::array-bounds.cpp` failure is due to a union that's acting 
> like an FAM. I have a question for you. Should `a` and `c` in the union in 
> this code be considered an FAM?

This test looks correct to me:

>   struct {
>     union {
>       short a[2]; // expected-note 4 {{declared here}}
>       char c[4];
>     };
>     int ignored;
>   };

I would **not** expect this to warn or trap because it's not the trailing 
member of the //structure//.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135727

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to