-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add fuzzer to bounds_of_expr_in_scope + fix discovered overflow bugs #5895
Conversation
…lex/add_bounds_fuzzing
…lex/add_bounds_fuzzing
src/Bounds.cpp
Outdated
@@ -257,7 +257,7 @@ class Bounds : public IRVisitor { | |||
bool could_overflow = true; | |||
if (to.can_represent(from) || to.is_float()) { | |||
could_overflow = false; | |||
} else if (to.is_int() && to.bits() >= 32) { | |||
} else if (to.is_int() && to.bits() > 16 && !(from.is_uint() && from.bits() > 16)) { | |||
// If we cast to an int32 or greater, assume that it won't |
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 comment now only applies to floats
Failures seem to be due to the uint32 -> int32 "fix", investigating now |
Update: the second fix broke |
The failures are also timeouts on the correctness tests. The added test takes ~15-20s on my machine - perhaps I should drop the test iteration count to 100, from 1000? @abadams |
100 is better, but the timeouts are actually unrelated (something's up with one of the cuda tests under llvm 13). Good to land. |
…alide#5895) * add interval bounds fuzzer * correct overflow checks in bounds inference * catch uint32->int32 overflow in simplifier and revert bounds change
This PR adds fuzz testing (based off of fuzz_simplify.cpp), and fixes a few of the overflow bugs that it discovers. Addresses #4724. The two examples below are results from the fuzzer:
->
This bug is the result of improper overflow handling for
UInt(32)
, as these values can be labelled as infinite by bounds inference, but have defined overflow semantics.->
This bug is a result of inconsistent handling of
UInt(32) -> Int(32)
casts, which has defined behavior in the simplifier but was not properly handled in bounds inference.Both bugs were fixed, and repeated running of the fuzzer has not revealed more inconsistencies yet.
While writing this fuzzer, I noticed that a lot of the fuzzers share code, and I think it would be very useful to have a shared set of fuzzing code, so I opened #5894 to make this a TODO.