ECM segfaults in Cygwin64

[embray]

Running ECM with almost any inputs results in a segfault on Cygwin64.

This is because any time it enters one of its hand-written assembly routines it needs to use the appropriate calling convention (Microsoft x64 convention on Windows), but it is only picking the correct Windows calling convention when compiling with mingw. It should do the same with Cygwin as well.

One possible workaround without patching would be to configure ECM with --disable-asm-redc to disable the custom assembly code entirely. But this is also overkill--the code works just fine as long as it's configured correctly for the platform.

Submitted upstream at ​https://gforge.inria.fr/tracker/index.php?func=detail&aid=20697&group_id=135&atid=623

[embray]

New commits:

​c80277a

Make sure the WINDOWS64_ABI flag is set on Cygwin as well, otherwise the assembly routines use the wrong calling convention.

[leif]

ECM 7.x got "revived", see #20385. 7.0.3 is still kind of beta/rc though I'd say.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​6361e6c

Bump patch level

[leif]

... as is the branch on #20385; i.e., not really "needs review" yet, but you could try whether that has been fixed upstream already, probably with vanilla 7.0.3.

[leif]

Replying to leif:

you could try whether that has been fixed upstream already, probably with vanilla 7.0.3.

Nope, in 7.0.3 (now configure.ac) there's still:

if test "x$enable_asm_redc" = xyes; then
# do the necessary definitions and includes
  AC_DEFINE([USE_ASM_REDC],1,[Define to 1 to use asm redc])
  test "x$CCAS" != x || CCAS="$CC -c"
  AC_SUBST([CCAS])
  GMP_PROG_M4
  GMP_ASM_UNDERSCORE
  GMP_ASM_TEXT
  GMP_ASM_GLOBL
  GMP_ASM_TYPE

  case $host in
    *-*-mingw32) GMP_DEFINE([WINDOWS64_ABI], 1)
                 AC_DEFINE([WINDOWS64_ABI], 1,[Define to 1 if x86_64 mulredc*() functions should be called with Windows ABI]);;
    *) ;;
  esac

  case $host in
    pentium3-*-*)
      echo "WARNING: Your processor is recognized as Pentium3."
      echo "         The asm code uses SSE2, and therefore it might"
      echo "         fail if your proc is indeed a P3, and not a"
      echo "         Pentium M. If you have compilation problems,"
      echo "         consider using --disable-asm-redc." ;;
    *)
  esac
fi
AM_CONDITIONAL([ENABLE_ASM_REDC], [test "x$enable_asm_redc" = xyes])

[leif]

I'll take care of including this patch on #20385 if upstream won't include it soon.

[embray]

Okay, thanks.

[jdemeyer]

1. This should be reported upstream.

2. You need to patch configure.in before configure otherwise the build system will want to run autoconf to update configure.

3. I cannot test this, but the patch is clear. If you say this makes ECM work on Cygwin64, that's good enough for me.

[leif]

Replying to jdemeyer:

2. You need to patch configure.in before configure otherwise the build system will want to run autoconf to update configure.

Oh right, patch doesn't by default take the modification time from the patch header (where configure is about a minute newer).

While it would be safer to explicitly touch configure after patching in spkg-install, make should^TM treat configure up-to-date if you just reverse the order in the patch. (I'm not even sure it wouldn't as is; you'd probably need a very slow machine to cause it to not do so.)

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​a4adca5

Change the patch order so that configure gets touched after configure.in

[jdemeyer]

I assume you actually tested this on Cygwin64 :-)

[leif]

Jeroen was quicker, but I also didn't test this on Cygwin[64]...

[embray]

Yup. Before this patch, running ecm segfaults. Without it, no segfaults (and all the tests that invoke it pass)

[leif]

FWIW, I guess similar would still happen on Cygwin32 on some x86 CPUs at least (probably x86_64 in 32-bit mode as well), but only if one explicitly configures GMP-ECM with --enable-asm-redc, which we don't do in Sage. (A user could still do that in Sage's environment variable ECM_CONFIGURE though.) The relevant asm code is in athlon/ and pentium4/.

Not sure whether the NTT inline assembly code (which is only used on [some] x86 CPUs, and again probably also on x86_64 in 32-bit mode) is safe w.r.t. the Windows ABI. (It might get used on Cygwin32, even by default, depending on the CPU, AFAICS.)

But both is IMHO beyond this ticket, since the ticket's title mentions just Cygwin64. (Feel free to try and eventually open a follow-up ticket for that... :P -- I personally won't.)

[embray]

The solution in the ticket just targets "cygwin" so it's a moot point I think.

Also I'm personally only targeting Sage for Cygwin64. Reason being I hope soon (once most of the issues are worked out) to release a Windows installer for Sage that includes cygwin as a DLL. This is for users, less so developers, and I'm not interested in non-64-bit platforms (though willing to poke at them if absolutely necessary).

[leif]

Replying to embray:

The solution in the ticket just targets "cygwin" so it's a moot point I think.

Nope, that doesn't fix the two issues I mentioned above, as on 32-bit systems the x86_64 code isn't used, and (of course) only in the latter WINDOWS64_ABI is used at all. (There's no macro for Windows ABI in general, nor Win32 ABI, although native Microsoft Windows -- with MS Visual Foo -- is handled.)

Also I'm personally only targeting Sage for Cygwin64. Reason being I hope soon (once most of the issues are worked out) to release a Windows installer for Sage that includes cygwin as a DLL. This is for users, less so developers, and I'm not interested in non-64-bit platforms (though willing to poke at them if absolutely necessary).

Ok.

Other Sage-on-Cygwin developers may try. (You could still detail your upstream report though, as it effectively only addresses Cygwin64.)

[embray]

I'm not really sure what you're trying to say here.

[leif]

Replying to embray:

I'm not really sure what you're trying to say here.

On 32-bit systems (including Cygwin32), other assembly files are used, which may cause similar problems (because they at least don't care about Pascal calling convention).

There's also some inline assembly which is used on 32-bit (x86) systems only; there I'm not sure whether it complies also to the Win32 ABI w.r.t. register usage.

[embray]

Ah, okay. That I don't know. If it is a problem it can be addressed in a new ticket.

[leif]

Replying to embray:

Ah, okay. That I don't know. If it is a problem it can be addressed in a new ticket.

Sure. In any case, it's easy to work around potential problems on Cygwin32 by simply setting ECM_CONFIGURE="--disable-sse2 --disable-asm-redc" before or when (re)installing the package.

Who is likely to test on Cygwin32?

[zimmerma]

patch applied upstream. Are you sure this is fine under 32-bit cygwin too?

Paul

[jpflori]

I should be able to test Cygwin32 this afternoon.

[zimmerma]

I should be able to test Cygwin32 this afternoon.

great! Please test revision 2976 (or later) of GMP-ECM:

$ svn checkout ​svn://scm.gforge.inria.fr/svnroot/ecm/trunk ecm

Paul

[leif]

Replying to jpflori:

I should be able to test Cygwin32 this afternoon.

I replied to Paul on the upstream bug report. (The patch here won't break anything on Cygwin32, but it won't solve potential similar issues on 32-bit Cygwin either.)

You may try trunk (outside of Sage) with --enable-sse2 (but without --enable-asm-redc), perhaps running make longcheck instead of just make check, and afterwards with --enable-asm-redc (but without --enable-sse2).

[leif]

Replying to leif:

Replying to jpflori:

I should be able to test Cygwin32 this afternoon.

You may try trunk (outside of Sage) with --enable-sse2 (but without --enable-asm-redc), perhaps running make longcheck instead of just make check, and afterwards with --enable-asm-redc (but without --enable-sse2).

Also saving the config.logs and the build logs perhaps, to see what actually got used. (In 6.4.4, there's 32-bit asm code in athlon/ and pentium4/, besides inline assembly in {ntt_gfp,spv}.c, which is less critical I think. Just noticed ECM 7.x now also has x86/, which I haven't at all looked at yet.)

[leif]

Replying to leif:

(In 6.4.4, there's 32-bit asm code in athlon/ and pentium4/, besides inline assembly in {ntt_gfp,spv}.c, which is less critical I think. Just noticed ECM 7.x now also has x86/, which I haven't at all looked at yet.)

Oh, there's no assembly code in x86/, just params.h:

#define MPZMOD_THRESHOLD 135
#define REDC_THRESHOLD 200
#define MPN_MUL_LO_THRESHOLD_TABLE {0, 0, 1, 1, 0, 0, 5, 1, 0, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 16, 1, 14, 14, 16, 16, 1}
#define NTT_GFP_TWIDDLE_DIF_BREAKOVER 11
#define NTT_GFP_TWIDDLE_DIT_BREAKOVER 11
#define MUL_NTT_THRESHOLD 262144
#define PREREVERTDIVISION_NTT_THRESHOLD 128
#define POLYINVERT_NTT_THRESHOLD 65536
#define POLYEVALT_NTT_THRESHOLD 16384
#define MPZSPV_NORMALISE_STRIDE 512

(And there's of course also inline assembly in longlong.h; don't know what's used on Cygwin[32], but presumably less critical, too.)

[jpflori]

Some reports using ECM trunk. On a x86_64, with a Cygwin64 setup, outside of Sage, using my own GMP 6.1.1 (dll) and Cygwin's GCC:

./configure --prefix=$LOCAL --with-gmp=$LOCAL --enable-shared --disable-static

I get

...
configure: Build for host type x86_64-unknown-cygwin
configure: CC=gcc, CFLAGS=-W -Wall -Wundef -O2 -pedantic -fomit-frame-pointer -m64 -mtune=sandybridge -march=sandybridge
configure: Linking GMP with -lgmp
configure: Using asm redc code from directory x86_64
configure: Not using SSE2 instructions in NTT code
configure: Using APRCL to prove factors prime/composite
configure: Assertions enabled
configure: Shell command execution disabled
configure: OpenMP disabled
configure: Memory debugging disabled

and make fails with

m4 -I../ -DOPERATION_mulredc1 `test -f mulredc1.asm || echo './'`mulredc1.asm >mulredc1.s
/bin/sh ../libtool    --mode=compile gcc  -O2 -pedantic -fomit-frame-pointer -m64 -mtune=sandybridge -march=sandybridge -c -o mulredc1.lo mulredc1.s
libtool: compile:  gcc -O2 -pedantic -fomit-frame-pointer -m64 -mtune=sandybridge -march=sandybridge -c mulredc1.s  -DDLL_EXPORT -DPIC -o .libs/mulredc1.o
/usr/lib/gcc/x86_64-pc-cygwin/5.4.0/../../../../x86_64-pc-cygwin/bin/as: BFD assertion (GNU Binutils) 2.25.2 a échoué /cygdrive/i/szsz/tmpp/cygwin64/binutils/binutils-2.25-4.x86_64/src/binutils-gdb/bfd/coff-x86_64.c:695
mulredc1.s: Messages de l'assembleur:
mulredc1.s:59: Erreur: ne peut représenter le type de réadressage BFD_RELOC_386_GOTPC

Windows does not like $_GLOBAL_OFFSET_TABLE I would say.

Using ECM 6.4.4 on my Cygwin64 setup with the patch to use Win64 ABI, I get a similar configuration and make is fine. Surely WANT_ASSERT gets set in one case and not in the other, leading to the invalid use of the GOT in the trunk version. But then make check fails with

./test.pp1 ./ecm.exe
Cygwin runtime failure: /home/jp/ecm-6.4.4/.libs/ecm.exe: Invalid relocation.  Offset 0x3025f223a at address 0x100414542 doesn't fit into 32 bits
############### ERROR ###############
Expected return code 8 but got 127
make: *** [Makefile:1982: check] Error 1

Going back to the trunk version and passing --disable-assert, make is fine. And make check runs but fail all tests.

FAIL: test.pp1
FAIL: test.pm1
FAIL: test.ecm

with the same relocation errors as in 6.4.4 as the log files tell.

On the same x86_64 machine, with the trunk version, using a Cygwin32 setup with a similar configure command, I get:

configure: Build for host type i686-pc-cygwin
configure: CC=gcc, CFLAGS=-W -Wall -Wundef -m32 -O2 -pedantic -fomit-frame-pointer -mtune=sandybridge -march=sandybridge
configure: Linking GMP with -lgmp
configure: Not using asm redc code
configure: Using SSE2 instructions in NTT code
configure: Using APRCL to prove factors prime/composite
configure: Assertions enabled
configure: Shell command execution disabled
configure: OpenMP disabled
configure: Memory debugging disabled

and make and make check are ok.

[jpflori]

Just realized the "relocation" errors might just be rebasing issues...

[embray]

Patch has been accepted upstream.

[jpflori]

Replying to jpflori:

Just realized the "relocation" errors might just be rebasing issues...

And I can confirm they are.

[jpflori]

I'll post further experiments on #20385.

[leif]

Or did I miss something?

[embray]

I'm not sure I understand the meaning--I took "in a later stable release" to mean there will eventually be a stable release that contains this fix. "Later" to me implies that it doesn't exist yet, but will.

[leif]

Replying to embray:

I'm not sure I understand the meaning--I took "in a later stable release" to mean there will eventually be a stable release that contains this fix. "Later" to me implies that it doesn't exist yet, but will.

No, "later" refers to what we currently have in Sage, so "fixed in a later [stable] upstream release" means there exists some upstream version without the issue, but Sage hasn't yet upgraded to it (which is quite often the case as some upgrades aren't straight-forward, for various reasons).

[leif]

So it's not as speculative as you thought... ;-)

[embray]

Okay, I've been misusing that then! Indeed I thought it was mostly speculative (but generally likely to be accurate for active projects). Anyways, fine by me.

[zimmerma]

a new release candidate is now available at

​https://members.loria.fr/PZimmermann/ecm-7.0.4-rc1.tar.gz

Paul