Karim Belabas on Tue, 04 Oct 2016 17:48:20 +0200


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

Re: is_universal_constant() assumes that gen_0 <= ghalf


Hi Jeroen,

* Jeroen Demeyer [2016-10-04 16:50]:
> Consider src/headers/pariinl.h which has
> INLINE int
> is_universal_constant(GEN x) { return (x >= gen_0 && x <= ghalf); }
> 
> This does not work correctly because the pointers gen_0 and ghalf come from
> two arrays: In src/language/init.c, there is one array for all constants
> except ghalf and one array for ghalf. How these arrays are layed out in
> memory is undefined behaviour. On Cygwin at least, it can happen that ghalf
> < gen_0 (with certain compilers and compiler flags) such that
> is_universal_constant(x) is always false. This leads to breakage in the Sage
> interface, involving closures returning gnil.

Thanks for the report: the bug is valid, but I'm not sure about the
proposed fix.

> The obvious fix is to ensure that all constants come from the same array,
> see attached patch.

0) You need to typecast to (ulong) instead of (long)  [ C++ compilers
will barf on this ]; easily fixed.

1) The name 'readonly_constants' is indeed fully declared after
its declarator and before its initializer so the code will compile with
both C and C++ compilers; this is good.

2) I don't think that we can rely on the address of 'readonly_constants'
to be already fixed within its initializer (even though it works in
practice with the compilers I tried); this is bad.

I modified your patch to define the *components* of ghalf in a previous
array. Thanks !

Cheers,

    K.B.
--
Karim Belabas, IMB (UMR 5251)  Tel: (+33) (0)5 40 00 26 17
Universite de Bordeaux         Fax: (+33) (0)5 40 00 21 23
351, cours de la Liberation    http://www.math.u-bordeaux.fr/~kbelabas/
F-33405 Talence (France)       http://pari.math.u-bordeaux.fr/  [PARI/GP]
`