[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

This revision was automatically updated to reflect the committed changes. Closed by commit rGda55e9ba1273: [Sema] Address-space sensitive index check for unbounded arrays (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8841-8844 +def warn_array_index_exceeds_max_addressable_bounds : Warning< + "array index %0

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked 11 inline comments as done. chrish_ericsson_atx added a comment. Addressed all feedback from Aaron, except for two comments about reachability that I don't understand. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 291184. chrish_ericsson_atx added a comment. Addressed feedback from Aaron Ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx added a comment. Thank you for the comments, @aaron.ballman . I'll update with the changes you requested shortly. I did have some requests for clarification of you, though. Thanks! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8841-8844

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

aaron.ballman added a comment. In D86796#2263557 , @chrish_ericsson_atx wrote: > Ping? Sorry for the delayed review, but this managed to fall off my radar. Thank you for the ping! Comment at:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx added a comment. Thanks for the excellent feedback, @ebevhan . @aaron.ballman , @krememek , or @rsmith , could one of you take a look at this change and if it's acceptable, please approve it? I have not requested commit privileges yet, either, so I will need your

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13981 +bool overflow; +llvm::APInt product(index); +product += 1; ebevhan wrote: > chrish_ericsson_atx wrote: > > ebevhan wrote: > > > What if index is wider than

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:14063 + if (isUnboundedArray) { +if (index.isUnsigned() || !index.isNegative()) { + const auto = getASTContext(); This could be early return to avoid the indentation.

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 289819. chrish_ericsson_atx added a comment. NC push did not resolve failed test. Rebased in hopes that whatever has broken the build has been resolved in the intervening commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 289815. chrish_ericsson_atx added a comment. NC. Pushing null change in hopes of re-triggering testing. Unit test that failed is low-level LLVM test, which doesn't even exercise the code I've changed here, so I'm assuming it's a spurious

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13966 if (index.isUnsigned() || !index.isNegative()) { -// It is possible that the type of the base expression after -//

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 289785. chrish_ericsson_atx added a comment. Refactored as ebevhan suggested to simplify patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13966 if (index.isUnsigned() || !index.isNegative()) { -// It is possible that the type of the base expression after -//

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 289753. chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added a comment. Addressed reviewer feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked 3 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13981 +bool overflow; +llvm::APInt product(index); +product += 1; ebevhan wrote: > What if index

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13981 +bool overflow; +llvm::APInt product(index); +product += 1; What if index is wider than AddrBits, but the active bits are fewer? Then you might miss out on

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13989 + MaxElems <<= AddrBits; + MaxElems /= ElemBytes; + ebevhan wrote: > The size calculations here could

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 289503. chrish_ericsson_atx added a comment. Updating D86796 : [Sema] Address-space sensitive index check for unbounded arrays Refactored math as suggested by Bevin Hansson. Repository: rG LLVM Github

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added a comment. I will tinker with the math to simplify as you suggest. Working with APInt and APSInt seems to promulgate sensitive and brittle code, which makes trying alternative expressions more tedious than I'd like

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

ebevhan added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13966 if (index.isUnsigned() || !index.isNegative()) { -// It is possible that the type of the base expression after -// IgnoreParenCasts is incomplete, even though the type of the base -//

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 288995. chrish_ericsson_atx added a comment. Corrected formatting (per git-clang-format) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx updated this revision to Diff 288649. chrish_ericsson_atx added a comment. Removed Change-Id from commit log message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

chrish_ericsson_atx created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. chrish_ericsson_atx requested review of this revision. Check applied to unbounded (incomplete) arrays and pointers to spot cases where the computed address is beyond the