chicken-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Chicken-hackers] this code looks wrong to me


From: Peter Bex
Subject: Re: [Chicken-hackers] this code looks wrong to me
Date: Thu, 18 Feb 2016 22:05:04 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Wed, Feb 17, 2016 at 01:53:00PM +0100, Jörg F. Wittenberger wrote:
> Stupid me.
> 
> The other way around attached.

Thanks.  This patch looked good to me, but I had a bad feeling that this
could go so wrong without being detected, so I wrote some tests.

Then I found that the whole thing is completely broken!  You can't use
C_peek_unsigned_integer by re-wrapping a direct pointer like that,
because it will use the stored object's data pointer as a C_word *,
which in the s32 case on a 64-bit machine means the s32 * will get cast
to a s64 *, causing wrong values to be returned and possibly segfaulting.

Attached is a proper patch which thankfully is shorter and simpler:
it simply casts the pointer to the correct type and converts the result
to a fixnum or flonum (or bignum on CHICKEN 5).

I simplified the f32 and f64 locative returning code to re-use the same
buffer that we use for the u32 and s32 (s64, u64) case.  We should
probably get rid of the ___tmpflonum stuff in other code too; it's
unnecessarily tricky and can be done a bit simpler.  Besides,
WORDS_PER_FLONUM is defined in runtime.c while C_alloc_flonum and
C_kontinue_flonum are defined in chicken.h; this is kind of bogus.
Anyway, something for later!

Cheers,
Peter

Attachment: 0001-Fix-references-into-u32-and-s32-locatives.chicken-5.patch
Description: Text Data

Attachment: 0001-Fix-references-into-u32-and-s32-locatives.master.patch
Description: Text Data

Attachment: signature.asc
Description: Digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]