[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-25 Thread Zoe Carver via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6201f6601dec: Check args passed to __builtin_frame_address and __builtin_return_address. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-25 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment. I was using `SemaBuiltinConstantArgRange` incorrectly. I was returning an error for `!SemaBuiltinConstantArgRange` instead of `SemaBuiltinConstantArgRange`. Also, I only modified the fail tests so I wasn't able to catch the error. Updated and am running both the Sema

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-25 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 246488. zoecarver added a comment. - diff from master (arg phab...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/lib/Sema/SemaChecking.cpp clang/t

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-25 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 246486. zoecarver added a comment. - Error if SemaBuiltinConstantArgRange returns true, not false Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/test/

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-24 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver reopened this revision. zoecarver added a comment. This revision is now accepted and ready to land. Reverted: 698078257285a044110620d7dab2fb4451a3fa29 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-24 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver closed this revision. zoecarver added a comment. Resolved by c93112dc4f745b0455addb54bfe1c2f79b827c6d Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-24 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 ___

[PATCH] D66839: Fix stack address builtin for negative numbers

2020-02-24 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 246277. zoecarver added a comment. - move verification to SemaChecking Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/lib/Sema/SemaChecking.cpp clan

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-12-16 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment. @Jim Thanks, I'll move it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-12-16 Thread Jim Lin via Phabricator via cfe-commits
Jim added a comment. I think this checking should be implemented in lib/Sema/SemaChecking.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 ___ cfe-commits m

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-08-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In the context of __builtin_frame_address, an arbitrary limit is probably okay. Maybe something like 0x, which is larger than anyone would realistically use, but doesn't take a crazy amount of time to compile. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-08-27 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment. Thanks for the explanation. Should I then not try to "fix" the issue? Or should I update sema checking? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-08-27 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. We usually prefer to generate error messages for incorrect parameters to builtins in SemaChecking.cpp. I don't think there's really an infinite loop here. The parameter to __builtin_frame_address is an unsigned integer; values greater than 0x7FFU aren't special.

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-08-27 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zoecarver added reviewers: kparzysz, eli.friedman. zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/test/Sema/builtin-stackaddress

[PATCH] D66839: Fix stack address builtin for negative numbers

2019-08-27 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/test/Sema/builtin-stackaddress.c:10 +// expected-error@+1 {{argument to '__builtin_return_address' must be a constant integer}} +return __builtin_return_address(x); }