unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* Serious bug in GUILE rational handling.
@ 2006-12-23 20:22 Han-Wen Nienhuys
  2006-12-23 20:55 ` Han-Wen Nienhuys
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Han-Wen Nienhuys @ 2006-12-23 20:22 UTC (permalink / raw)
  Cc: schottstaedt, Jan Nieuwenhuizen


Hello,

scm_equal_p inspects SCM_CELL_TYPE() of a rational. Cell type is also used
to hold the reduced bit. This leads to interesting behavior,

guile> (define x 1/2)
guile> (define y 1/2)
guile> (denominator x)
2
guile> (equal? x y)
#f
guile> 


This took a lot of effort to debug; printing the rational while
debugging would alter the reduction bit, making the problem go away,
suggesting memory corruption rather than a programming error.

Furthermore, I think

  if (!(SCM_FRACTION_REDUCED (z)))
    {
      SCM divisor;
      divisor = scm_gcd (SCM_FRACTION_NUMERATOR (z), SCM_FRACTION_DENOMINATOR (z));
      if (!(scm_is_eq (divisor, SCM_I_MAKINUM(1))))
	{
	  /* is this safe? */
	  SCM_FRACTION_SET_NUMERATOR (z, scm_divide (SCM_FRACTION_NUMERATOR (z), divisor));
	  SCM_FRACTION_SET_DENOMINATOR (z, scm_divide (SCM_FRACTION_DENOMINATOR (z), divisor));
	}
      SCM_FRACTION_REDUCED_SET (z);
    }

introduces a race condition.

Suppose another thread triggers scm_i_fraction_reduce at the same
time, and reads the partial result of the first scm_i_fraction_reduce.
This is really insidious, as the corruption would happen upon reading
the data, and will show up somewhere completely different.

I have removed support for the reduced bit, and put the reduction in
make_fraction.

I am about to commit these changes to CVS, and will also commit them
to the GUILE 1.8 branch.  I would appreciate it if a GUILE 1.8.2 would
be released ASAP.  I need working rational numbers in GUILE for
LilyPond 2.11.x, and it would be nice if I didn't have to force my
users to run GUILE from CVS.

--
 Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: Serious bug inn GUILE rational handling
@ 2006-12-24 11:32 Bill Schottstaedt
  2006-12-29  2:52 ` Neil Jerram
  0 siblings, 1 reply; 12+ messages in thread
From: Bill Schottstaedt @ 2006-12-24 11:32 UTC (permalink / raw)


> I have removed support for the reduced bit, and put the reduction in
> make_fraction.

I think it was intended that equal? would use scm_i_fraction_equalp
which reduces both arguments before checking equality.  So the
simplest fix would be to mask off the reduced bit in the cell type
in the check for cell type equality in scm_equalp.  I would hesitate
to remove support for this bit because it will mean you get gcd
on every integer divide!  The current system already slows Guile
down by about 10%.   On the race condition, my vage recollection
is that the "is this safe?" question was mine, and I hoped at that
time that someone who knew about such things would check it
out -- I believe (it's been a long time since I looked at this stuff)
that if that line is not safe, there are a lot more like it scattered
around Guile, so it's scarcely reason to jettison the entire thing.



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2007-01-03 21:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-23 20:22 Serious bug in GUILE rational handling Han-Wen Nienhuys
2006-12-23 20:55 ` Han-Wen Nienhuys
2006-12-23 23:02   ` Kevin Ryde
2006-12-23 23:21 ` Kevin Ryde
2006-12-24  0:59   ` Han-Wen Nienhuys
2006-12-25 21:33     ` Kevin Ryde
2006-12-24  1:06   ` Han-Wen Nienhuys
2006-12-24  9:43     ` Kevin Ryde
2007-01-03 21:35   ` Carl Witty
2006-12-23 23:41 ` Kevin Ryde
  -- strict thread matches above, loose matches on Subject: below --
2006-12-24 11:32 Serious bug inn " Bill Schottstaedt
2006-12-29  2:52 ` Neil Jerram
2006-12-29 12:39   ` Serious bug in " Bill Schottstaedt
2006-12-30 20:42     ` Neil Jerram

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).