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

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG283e0a81ef35: [clang] Correct sanitizer behavior in union FAMs (authored by void). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews

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

2022-10-20 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. This looks great to me. Thanks! 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 ht

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

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 469360. void added a comment. Add "real" FAM to testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews.llvm.org/D135727 Files: clang/lib/AST/Expr.cpp clang/test/CodeGen/bounds-che

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

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 469355. void added a comment. Update to support SemaCXX tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews.llvm.org/D135727 Files: clang/lib/AST/Expr.cpp clang/test/CodeGen/bound

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

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. @kees @serge-sans-paille I think this is ready for another go. PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews.llvm.org/D135727 ___ cfe-commits maili

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

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 469332. void added a comment. Update testcases to pass tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135727/new/ https://reviews.llvm.org/D135727 Files: clang/lib/AST/Expr.cpp clang/test/CodeGen/bounds

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

2022-10-13 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In D135727#3856978 , @void wrote: > I think we opened the can or worms. :-) At this point I think the can we opened has worms with cans of worms with cans of worms. I'd say it's turtles all the way down, but it seems to just be wor

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

2022-10-13 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. In D135727#3854252 , @kees wrote: > In 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 Clan

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

2022-10-12 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. In 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/arr

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

2022-10-12 Thread 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

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

2022-10-11 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/lib/AST/Expr.cpp:228 +if (StrictFlexArraysLevel == FAMKind::OneZeroOrIncomplete && Size.uge(2)) + return false; } else if (!Context.getAsIncompleteArrayType(getType())) I <3 this simplificatio

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

2022-10-11 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment. Change log typo? "but for all" should be "but not for all" ? Comment at: clang/lib/AST/Expr.cpp:223 // arrays to be treated as flexible-array-members, we still emit diagnostics // as if they are not. Pending further discussion... +if (Strict

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

2022-10-11 Thread Bill Wendling via Phabricator via cfe-commits
void created this revision. void added reviewers: kees, serge-sans-paille, rsmith, efriedma. Herald added a project: All. void requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Clang doesn't have the same behavior as GCC does with union flexi