Bug 4401 - cairo detects and uses buggy sincos() on Tru64, cairo_matrix_init_rotate
Summary: cairo detects and uses buggy sincos() on Tru64, cairo_matrix_init_rotate
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.0.0
Hardware: Alpha All
: high normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-09-08 15:31 UTC by Tim Mooney
Modified: 2007-01-23 20:05 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Remove use of sincos from cairo (1.06 KB, patch)
2005-09-08 16:30 UTC, Carl Worth
Details | Splinter Review

Description Tim Mooney 2005-09-08 15:31:41 UTC
cairo's configure detects that Tru64 has some symbol named sincos(), so it uses
it in cairo_matrix_init_rotate().  sincos isn't documented on Tru64 and I can't
find anything about it in any of the headers, so it's not even at the level of
"deprecated" or "obsolete" interface.

Using sincos() causes problems (SIGFPE, floating point exception) for tests like
transforms, and likely causes problems in real-world code too, because the
values that get filled into the cairo_matrix_t by cairo_matrix_init_rotate()
likely trigger underflow:


(ladebug) list
    560 }
    561 
    562 cairo_status_t
    563 _cairo_gstate_rotate (cairo_gstate_t *gstate, double angle)
    564 {
    565     cairo_matrix_t tmp;
    566 
    567     _cairo_gstate_unset_scaled_font (gstate);
    568     
    569     cairo_matrix_init_rotate (&tmp, angle);
>   570     cairo_matrix_multiply (&gstate->ctm, &tmp, &gstate->ctm);
    571 
    572     cairo_matrix_init_rotate (&tmp, -angle);
    573     cairo_matrix_multiply (&gstate->ctm_inverse, &gstate->ctm_inverse,
&tmp);
    574 
    575     return CAIRO_STATUS_SUCCESS;
    576 }
(ladebug) print angle
1.5707963267948966
(ladebug) print tmp
struct _cairo_matrix {
  xx = 2.172393021099427e-311;
  yx = 2.6525782160381117e-314;
  xy = -2.6525782160381117e-314;
  yy = 2.172393021099427e-311;
  x0 = 0;
  y0 = 0;
}

These values trigger the SIGFPE in the cairo_matrix_multiply() routine.

If I #undef HAVE_SINCOS and rebuild, so that separate calls to sin() and cos()
are made, the values filled in by cairo_matrix_init_rotate are much more
sensible, and the transforms test passes.

If it's a big win performance-wise to call sincos() instead of separate calls to
sin() and cos(), I can try find a way to augment the autoconf test for sincos()
so that it detects a nonfunctional sincos().  If it's not a big win, it might be
better to just use sin() and cos() all the time.

Let me know what you think the best approach is.
Comment 1 Carl Worth 2005-09-08 16:30:20 UTC
Created attachment 3203 [details] [review]
Remove use of sincos from cairo

Ouch.

I don't think we have any record that using sincos is a measurable
performance win. In the light of this bug, I favor removing its use.

Does the attached patch solve the problem for you?
Comment 2 Tim Mooney 2005-09-09 08:47:36 UTC
Verified.  With this patch, the "transforms" and "text-rotate" tests no longer
segfault (and they pass).
Comment 3 Carl Worth 2005-09-12 11:12:17 UTC
Thanks for verifying. Now fixed in CVS to appear in both 1.0.2 and 1.2.0:

2005-09-12  Carl Worth  <cworth@cworth.org>

        Fix for bug #4401 as reported by Tim Mooney:

        * configure.in: Don't bother checking for sincos function.

        * src/cairo-matrix.c: (cairo_matrix_init_rotate): Don't use sincos
        function since it is apparently buggy on some platforms, (Tru64 at
        least).


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.