Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upEnsure the result of calculation is finite number. #1296
Conversation
|
Thank you for the PR! |
| @@ -150,6 +150,15 @@ pub fn subtract_rect<U>(rect: &TypedRect<f32, U>, | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| pub fn ensure_float_is_normal(x: f32) -> f32 { | |||
This comment has been minimized.
This comment has been minimized.
kvark
May 26, 2017
Member
This is rather minor, since the function is only used once, but if we want to make it generally useful, I think we should have an API like get_normal(x: f32) -> Option<f32>, so that the function clients do unwrap_or(0.0) (or other value, depending on the context).
This comment has been minimized.
This comment has been minimized.
mephisto41
force-pushed the
mephisto41:ensure-numbers-arent-nan-or-infinite
branch
from
3dde1a7
to
517c172
|
@bors-servo r+ |
|
|
…, r=kvark Ensure the result of calculation is finite number. In this [test](https://searchfox.org/mozilla-central/source/dom/animation/test/crashtests/1272475-1.html), we create a very large transform which exceed max of float. This results many NaN or infinite numbers. If we don't handle it, the partial_cmp with Nan or infinite numbers would cause crash. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1296) <!-- Reviewable:end -->
|
|
|
@mephisto41 This PR appears to cause a panic in gecko crashtests, when running dom/animation/test/crashtests/1272475-1.html. Backtrace can be found at https://treeherder.mozilla.org/logviewer.html#?job_id=102467951&repo=try&lineNumber=2240 |
|
@staktrace interesting, so the only new call this PR makes is It doesn't appear to be unsafe to me, so I don't understand why build bots crash on it. |
|
Based on the line number in the stack, it seems to be coming from this line so most likely an underflow? |
|
Yes, see #1302 |
Overflowing protection for TransformedRect Fixes #1296 (comment) cc @staktrace @mephisto41 r? @glennw or anyone <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1302) <!-- Reviewable:end -->
Overflowing protection for TransformedRect Fixes #1296 (comment) cc @staktrace @mephisto41 r? @glennw or anyone <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1302) <!-- Reviewable:end -->
mephisto41 commentedMay 26, 2017
•
edited by larsbergstrom
In this test, we create a very large transform which exceed max of float. This results many NaN or infinite numbers. If we don't handle it, the partial_cmp with Nan or infinite numbers would cause crash.
This change is