[PATCH] D135727: [clang] Correct sanitizer behavior in union FAMs

[Bill Wendling via Phabricator via cfe-commits]

void marked 2 inline comments as done.
void added a comment.

@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.

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?

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



================
Comment at: clang/lib/AST/Expr.cpp:228
+    if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2))
+      return false;
   } else if (!Context.getAsIncompleteArrayType(getType()))
----------------
serge-sans-paille wrote:
> I <3 this simplification! This is definitively going to introduce regression, but if this aligns with GCC behavior I think it's okay.
> I don't see how it's related to union though. Could you explain?
The original code allows any terminating array over size one to not be considered a FAM. And so the conditional below (`if (FD->getParent()->isUnion())`) is never executed. The rest of this commit is mostly cleaning up the test cases. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135727