| Summary: | [PATCH] Recording surface ink extents: difference between two fixed point numbers may overflow before conversion to double | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Carlos Antunes <cmantunes> |
| Component: | general | Assignee: | Chris Wilson <chris> |
| Status: | RESOLVED MOVED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | 1.12.16 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Patch fixes fixed-point overflow bug on recording-surface
Simple test to exercise bug |
||
Do you happen to have a self-contained example program demonstrating the problem that you are fixing? Could you provide patches as git commits (google for git format-patch). I am not sure about the problem that you are fixing. If the extents are large enough to overflow the fixed point calculations, wouldn't other code (which also uses the fixed point numbers) possibly run into the same overflows? So while I agree that fixing this is a good idea, I am not sure that this actually means that it becomes safe to use a recording surface with large-ish coordinates. Created attachment 107786 [details]
Simple test to exercise bug
It's amazing that, after more than two years, this trivial to fix bug is still present in the code! -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/188. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 107759 [details] Patch fixes fixed-point overflow bug on recording-surface Recording surface may return incorrect result for ink extents when, for example, it is created unbounded. This is due to a bug in the conversion of a difference from fixed to floating point. The code for cairo_recording_surface_ink_extents reads at some point: *width = _cairo_fixed_to_double (bbox.p2.x - bbox.p1.x); *height = _cairo_fixed_to_double (bbox.p2.y - bbox.p1.y); The "solution" (short of getting rid of fixed point altogether) is to change this to: *width = _cairo_fixed_to_double (bbox.p2.x) - _cairo_fixed_to_double (bbox.p1.x); *height = _cairo_fixed_to_double (bbox.p2.y) - _cairo_fixed_to_double (bbox.p1.y); This was tested on MSYS2/MinGW64 and indeed it works as expected. Attached, you'll find the simple patch. It works against 1.12.16 but the bug is still present on HEAD. Thanks!