-
Notifications
You must be signed in to change notification settings - Fork 13.4k
trans: Use an isize to count the number of registers so we don't underflow for fn's with > 7 args in debug builds. #29091
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ |
📌 Commit 8719e94 has been approved by |
@luqmana btw, were you testing/developing this in a debug build? (Probably not since bootstrapping debug-builds is currently hosed.) I ask because this seems like a case we could add to the list of "bugs that would be caught by overflow/underflow checking arithmetic"... |
⌛ Testing commit 8719e94 with merge 02ffb5f... |
💔 Test failed - auto-mac-64-opt |
// a, b, c, d should be in registers | ||
// e, f should be on the stack | ||
// s should be byval pointer on the stack | ||
void byval_many_rect(int32_t a, int32_t b, int32_t c, int32_t d, int32_t e, int32_t f, struct Rect s) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line failed tidy (> 100 chars).
…rflow for fn's with > 7 args in debug builds.
8719e94
to
82f08ea
Compare
@bors r=pnkfelix |
📌 Commit 82f08ea has been approved by |
@pnkfelix Yea, it was with just |
⌛ Testing commit 82f08ea with merge 92af6a1... |
💔 Test failed - auto-mac-64-opt |
Looks like this timed out. |
@bors retry
|
⌛ Testing commit 82f08ea with merge cbdd5ac... |
💔 Test failed - auto-linux-64-opt |
It doesn't seem like the test failure is related to this PR. I was able to compile and run all tests successfully on my local 64-bit Linux machine with optimizations enabled. |
@bors retry |
Fixes #29073.