Kartikaya Gupta (email:kats@mozilla.staktrace.com) Reporter
	Description • 9 years ago

+++ This bug was initially created as a clone of Bug #1135677 +++

Bug 1135677 was filed to fix a specific issue causing a crashtest failure. A "safe" localized patch was used for that bug as it needed uplifting. This bug is to fix the more general problem.

The general problem is that given a rect with a large x and width values, XMost() will overflow and return a negative number. If anything uses this negative number it could propagate the error. When BenWa and I discussed it he suggested a good fix would be to clamp XMost() so that it never overflows.

I had pushed a try push with a WIP that partially implemented this, with a MOZ_ASSERT to see how many tests are running into this scenario. The try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=53459b7da609 - seems to be mostly the one crashtest that's having this problem.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com) Reporter
	Comment 1 • 9 years ago

This is the approach I was taking, but the tests I wrote were failing because I'm assuming a wrap-around overflow when in fact overflow is undefined (and even with wrap-around it doesn't work for floats). So froydnj pointed me to mozilla/CheckedInt.h but we probably can't use that directly in BaseRect. Instead as Botond suggested we'd need to have some templated class like CoordTraits which can do overflow detection on different numeric types reliably. At this point this seems too involved for me to tackle right now, so I'm parking this patch here. Anybody else interested in picking it up should feel free, otherwise I'll get back to it in the future sometime.