Subject: Re: bug#13030: factor: infinite loop on Linux/powerpc

From: Pádraig Brady <[email-masked]>
Date: Fri, 04 Jan 2013 14:47:06 +0000

On 11/29/2012 08:08 PM, Pádraig Brady wrote:
> On 11/29/2012 07:36 PM, Torbjorn Granlund wrote:
>> Pádraig Brady <[email-masked]> writes:
>>
>>    > -#if HAVE_HOST_CPU_FAMILY_powerpc && W_TYPE_SIZE == 64
>>    > +#if HAVE_HOST_CPU_FAMILY_powerpc && W_TYPE_SIZE == 64 && defined (_LP64)
>>    >   #if !defined (_LONG_LONG_LIMB)
>>    >   /* _LONG_LONG_LIMB is ABI=mode32 where adde operates on 32-bit values.  So
>>    >      use adde etc only when not _LONG_LONG_LIMB.  */
>>    >
>>    > Thanks,
>>    >
>>
>> I suppose it might be much better to make W_TYPE_SIZE not be set to a
>> size not supported by the present ABI.
>>
>> That way, we will avoid longlong.h divergence.
>
> So that's a bit in contradiction to:
> http://bugs.gnu.org/12754#20
>
> So you're suggesting that this would be safest?
>
> diff --git a/src/factor.c b/src/factor.c
> index 6d1d17a..d2a4158 100644
> --- a/src/factor.c
> +++ b/src/factor.c
> @@ -126,11 +126,11 @@
>   /* Make definitions for longlong.h to make it do what it can do for us */
>
>   /* bitcount for uintmax_t */
> -# if UINTMAX_MAX == UINT32_MAX
> +# if ULONG_MAX == UINT32_MAX
>   #  define W_TYPE_SIZE 32
> -# elif UINTMAX_MAX == UINT64_MAX
> +# elif ULONG_MAX == UINT64_MAX
>   #  define W_TYPE_SIZE 64
> -# elif UINTMAX_MAX == UINT128_MAX
> +# elif ULONG_MAX == UINT128_MAX
>   #  define W_TYPE_SIZE 128
>   # endif
>
> In this particular case that would enable
> the 32 bit ppc assembly in longlong.h

If you do the above, you hit run time assertions like:

$ factor 2123123123123123123123
factor: factor.c:974: mulredc2: Assertion `(a1 >> (32 - 1)) == 0' failed.
32767000000000000000000:Aborted

which makes sense as we're using uintmax_t throughout factor.c.
But something general is required here rather than fixing
up every architecture. I've just noticed that sparc v9 in 32 bit mode
will also loop for ever with the above command with what's in
place in longlong.h at present.

So what I'm going to do is pull all the _LP64 amendments to longlong.h
and instead avoid it completely with this in factor.c:

diff --git a/src/factor.c b/src/factor.c
index 473eee7..95451a5 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -118,7 +118,14 @@
  #endif

  #ifndef USE_LONGLONG_H
-# define USE_LONGLONG_H 1
+/* With the way we use longlong.h, it's only safe to use
+   when UWtype = UHWtype, as there were various cases
+   (as can be seen in the history for longlong.h) where
+   for example, _LP64 was required to enable W_TYPE_SIZE==64 code,
+   to avoid compile time or run time issues.  */
+# if LONG_MAX == INTMAX_MAX
+#  define USE_LONGLONG_H 1
+# endif
  #endif

  #if USE_LONGLONG_H

That works for i686 x86_64 and sparcv7 and sparcv9 at least.

thanks,
Pádraig.