Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#20385 closed enhancement (fixed)

Update GMP-ECM to version 7.0

Reported by: zimmerma Owned by:
Priority: major Milestone: sage-7.4
Component: packages: standard Keywords:
Cc: fbissey Merged in:
Authors: Leif Leonhardy, Jeroen Demeyer, François Bissey, Antonio Rojas Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: 946c580 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by fbissey)

GMP-ECM 7.0.4 is available at https://gforge.inria.fr/frs/download.php/file/36224/ecm-7.0.4.tar.gz.


(This was referring to version 7.0:)

Some issues were reported for the Debian package:

https://buildd.debian.org/status/package.php?p=gmp-ecm, in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?


Upgrade GMP-ECM to 7.0.4

  • I haven't repackaged the upstream tarball (.tar.gz) [yet]. Not sure whether we could/should also delete some stuff not needed for Sage (cf. SPKG.txt); at least there's no spkg-src script.

  • I haven't touched spkg-install yet. We may address potential issues with SAGE_FAT_BINARY (mentioned both there and in SPKG.txt), as well as known problems with -march=native -O3 on bdver[1-4] (AMD FX-???? Bulldozer/Piledriver, AMD Opteron 6[23]xx, etc.) where we should add -mno-xop because of bugs in (AFAIK) every GCC version that supports these processors, silently leading to broken code ("just" testsuite failures IIRC). Also, we currently copy Sage's config.guess (because it was newer at some point), which presumably is no longer necessary (but shouldn't hurt either, provided we keep Sage's up-to-date and GMP-ECM won't rely on its own).

  • The doctests in sage/libs/libecm.pyx may need some changes, especially with respect to verbose mode tests. (But I'll presumably work on them elsewhere anyway, independently of this upgrade.)
  • We definitely also have to fix the pexpect interface to ecm in src/sage/interfaces/ecm.py, as the command line interface has changed in GMP-ECM 7.x.

Attachments (2)

ecm.log (56.2 KB) - added by fbissey 4 years ago.
ecm log - bare spkg-install run
config.log (123.3 KB) - added by fbissey 4 years ago.
config.log matching ecm.log

Download all attachments as: .zip

Change History (171)

comment:1 Changed 4 years ago by fbissey

  • Description modified (diff)

comment:2 Changed 4 years ago by fbissey

  • Milestone changed from sage-7.2 to sage-7.4

comment:3 Changed 4 years ago by leif

  • Component changed from elliptic curves to packages: standard

Cool!

comment:4 in reply to: ↑ description Changed 4 years ago by leif

Replying to zimmerma:

Some issues were reported for the Debian package: https://buildd.debian.org/status/package.php?p=gmp-ecm, in particular on 32-bit PowerPC and FreeBSD. Is there any such machine on the Sage build farm?

I fail to see failures there by now (the version listed there also is 7.0.3+ds-1 though).

And no, AFAICT we don't have any PowerPC (neither 32-bit nor 64-bit) nor any FreeBSD buildbot.

I guess François regularly tests on Linux PowerPC64 though, and I think Jeroen still has access to some PowerPC running MacOS X 10.5 (we might no longer support; AFAIK 32-bit OS, not sure, but 64-bit CPU).

comment:5 Changed 4 years ago by leif

  • Description modified (diff)

"Upgraded" to version 7.0.3 (of July 4th, currently the latest available).

(Strange that this version and 7.0.2 apparently haven't been tagged in the svn repo.)

comment:6 Changed 4 years ago by leif

According to Paul's comment, we can drop the only patch (from #19233) we currently have in Sage.

comment:7 follow-up: Changed 4 years ago by leif

Should we repackage the tarball with bzip2?

comment:8 in reply to: ↑ 7 Changed 4 years ago by leif

Replying to leif:

Should we repackage the tarball with bzip2?

900K	ecm-7.0.3.tar.bz2
1.1M	ecm-7.0.3.tar.gz

comment:9 Changed 4 years ago by leif

  • Authors set to Leif Leonhardy
  • Branch set to u/leif/gmp-ecm-7.x
  • Commit set to c0f78464d52f731e6fcb4486c281c93bf76bcdbc
  • Description modified (diff)
  • Status changed from new to needs_review

Note that the patch is preliminary (cf. the commit message / ticket description), but I'm setting it to "needs review" as there's something that can already be tested.


New commits:

c0f7846GMP-ECM: #20385: Upgrade package to 7.0.3 (preliminary patch, see below)

comment:10 in reply to: ↑ description Changed 4 years ago by leif

Replying to leif's ticket description:

  • I haven't touched spkg-install yet. We may address [...] known problems with -march=native -O3 on bdver[1-4] (AMD FX-???? Bulldozer/Piledriver, AMD Opteron 6[23]xx, etc.) where we should add -mno-xop because of bugs in (AFAIK) every GCC version that supports these processors, silently leading to broken code ("just" testsuite failures IIRC).

This is still true for 7.0.3 (and Ubuntu's GCC 4.6.3 at least), while what used to be make check is now make longcheck:

...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Input number is 2277189375098448170118558775447117254551111605543304035536750762506158547102293199086726265869065639109 (103 digits)
Using B1=1000000, B2=0, polynomial x^1, x0=3
Step 1 took 584ms
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
############### ERROR ###############
Expected return code 14 but got 0
make: *** [longcheck] Error 1

I think we should do make check && make longcheck in spkg-check then.

comment:11 Changed 4 years ago by leif

Ooops, on the other hand make check now fails with -march=native -mno-xop -O3 -g (while it passed without -mno-xop?!?!):

FAIL: test.pp1
PASS: test.pm1
PASS: test.ecm
============================================================================
Testsuite summary for ecm 7.0.3
============================================================================
# TOTAL: 3
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See ./test-suite.log
Please report to ecm-discuss@lists.gforge.inria.fr
============================================================================
make[3]: *** [test-suite.log] Error 1
make[3]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make[2]: *** [check-TESTS] Error 2
make[2]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/home/leif/build/ecm-7.0.3-build.bulldozer-gcc-4.6.3-default'
make: *** [check-recursive] Error 1

test-suite.log:

...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Input number is 2277189375098448170118558775447117254551111605543304035536750762506158547102293199086726265869065639109 (103 digits)
Using B1=2337233, B2=173055082, polynomial x^1, x0=3
Step 1 took 1368ms
Step 2 took 104ms
********** Factor found in step 2: 4190453151940208656715582382315221647
Found prime factor of 37 digits: 4190453151940208656715582382315221647
Prime cofactor 543423179434447039008165356160798838947285203071935410761431031147 has 66 digits
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc] [P+1]
Save file test.pp1.save already exists, will not overwrite
############### ERROR ###############
Expected return code 0 but got 1
FAIL test.pp1 (exit status: 1)

(make longcheck fails in the same way as it apparently also performs the same test(s).)

comment:12 Changed 4 years ago by leif

Hmmm, or maybe not, and make clean was just not enough.

After make distclean, make check again passes with -march=native -mtune=native -mno-xop -O3 -g in gmp.h (but now also --enable-assert).

But in make longcheck, I'm now getting a different, apparently unrelated error:

...
GMP-ECM 7.0.3 [configured with GMP 5.1.1, --enable-asm-redc, --enable-assert] [P+1]
Input number is 18446744073709551337 (20 digits)
Using B1=70823, B2=1320588, polynomial x^1, x0=2
Step 1 took 16ms
********** Factor found in step 1: 18446744073709551337
Found input number N
../../src/ecm-7.0.3/test.pp1: 151: ../../src/ecm-7.0.3/test.pp1: cannot open ./c155: No such file
############### ERROR ###############
Expected return code 0 but got 2
make: *** [longcheck] Error 1

comment:13 Changed 4 years ago by git

  • Commit changed from c0f78464d52f731e6fcb4486c281c93bf76bcdbc to 64eb69eca4d560773a8057b533a341f18c5ab3cf

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

64eb69eGMP-ECM: #20385: (Also) run 'make longcheck' in spkg-check

comment:14 follow-up: Changed 4 years ago by leif

It seems make longcheck needs some upstream work, as I'm also getting (ignored!) "meta-errors" (syntax errors etc.) with a clean, fresh build on another (non-AMD) machine.

comment:15 in reply to: ↑ 14 ; follow-up: Changed 4 years ago by leif

Replying to leif:

It seems make longcheck needs some upstream work, as I'm also getting (ignored!) "meta-errors" (syntax errors etc.) with a clean, fresh build on another (non-AMD) machine.

Apparently also building out-of-tree is broken (more precisely, running make longcheck in an out-of-tree build, which definitely fails).

comment:16 follow-up: Changed 4 years ago by fbissey

Looks like I wasn't on cc even so Paul explicitly added me when the ticket was created. I missed all your action from the last few hours. My gentoo ebuild passes "make check" but then I don't do fancy flags. Will try "make longcheck". Before the release Paul opened a conversation on the last ecm upgrade ticket. Apart from the doctest there is interface change to deal with https://trac.sagemath.org/ticket/14151#comment:46

comment:17 in reply to: ↑ 16 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

Looks like I wasn't on cc even so Paul explicitly added me when the ticket was created. I missed all your action from the last few hours.

Hmmm, perhaps recheck your trac notification preferences as you already commented on this ticket, too (so you should by default receive notifications even when not explicitly cc'ed).


My gentoo ebuild passes "make check" but then I don't do fancy flags. Will try "make longcheck". Before the release Paul opened a conversation on the last ecm upgrade ticket. Apart from the doctest there is interface change to deal with https://trac.sagemath.org/ticket/14151#comment:46

And although I participated in that old ticket, I somehow missed all that discussion from a couple of months ago...

I have to admit I totally forgot Sage's pexpect interface to ecm.

Is the execstack issue meanwhile resolved upstream?

comment:18 in reply to: ↑ 17 ; follow-up: Changed 4 years ago by fbissey

Replying to leif:

Replying to fbissey:

Is the execstack issue meanwhile resolved upstream?

Nope. I have added myself to cc because I din't get your last message either.

comment:19 Changed 4 years ago by git

  • Commit changed from 64eb69eca4d560773a8057b533a341f18c5ab3cf to 0affd290f491cf624f15fec080f3b349bd07477c

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

0affd29GMP-ECM: #20385: Restore note on work-around in SPKG.txt that accidentally got dropped in the 6.4.4 package.

comment:20 Changed 4 years ago by leif

  • Cc fbissey added

Hope you get this now, as the cc field was still empty...

comment:21 Changed 4 years ago by leif

  • Description modified (diff)

Added command line interface change to be addressed.

comment:22 in reply to: ↑ 18 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

Replying to leif:

Is the execstack issue meanwhile resolved upstream?

Nope.

Should we add -Wl,-z,noexecstack in spkg-install until that's fixed upstream?

comment:23 in reply to: ↑ 22 ; follow-up: Changed 4 years ago by fbissey

Replying to leif:

Replying to fbissey:

Replying to leif:

Is the execstack issue meanwhile resolved upstream?

Nope.

Should we add -Wl,-z,noexecstack in spkg-install until that's fixed upstream?

Yes. Upstream is open to a patch. Problem is the files are generated so you have to carefully fix the generator :(

Still no notification, I'll have to inspect the spam filter tomorrow but I suspect it is not even making it there. May have to change the email address for my notification. And go to the dashboard to see if I missed updates on other tickets.

comment:24 Changed 4 years ago by fbissey

My goodness! notification this time.

comment:25 Changed 4 years ago by leif

So after N attempts, I finally built GMP-ECM in-place two times (separate folders) on a Bulldozer (Opteron 6276), once with -march=native -mtune=native -O3 -g, and once with -march=native -mtune=native -mno-xop -O3 -g.

In both cases, make check and make longcheck succeeded now.

(This presumably means the relevant code in GMP-ECM 6.x has changed enough to no longer expose the bugs in GCC when mixing XOP and AVX instructions at higher optimization levels. That's not too surprising, as even minor changes made the issue vanish when I tried (hard!) to create a testcase for bugzilla.)

comment:26 in reply to: ↑ 23 Changed 4 years ago by leif

Replying to fbissey:

Problem is the files are generated so you have to carefully fix the generator :(

? LDFLAGS? Perhaps CCASFLAGS?

Actually, upstream should add a configure check and add the appropriate flags when supported.

Do we have to care about anything but Linux (x86/x86_64/ppc/ppc64/SPARC...)?

Anything but GNU ld? (Yes, on Solaris, the Sun linker may get used, but Solaris' not Linux.)

comment:27 Changed 4 years ago by fbissey

LDFLAGS, linux x86(_64) only I think. As for ld I still suffer AIX boxes on ppc64 but sage won't build over there anyway (building python packages need hand holding for starter).

comment:28 Changed 4 years ago by leif

GMP (6.1.1) has this:

dnl Checks whether the stack can be marked nonexecutable by passing an option
dnl to the C-compiler when acting on .s files. Appends that option to ASMFLAGS.
dnl This macro is adapted from one found in GLIBC-2.3.5.
dnl FIXME: This test looks broken. It tests that a file with .note.GNU-stack...
dnl can be compiled/assembled with -Wa,--noexecstack.  It does not determine
dnl if that command-line option has any effect on general asm code.
AC_DEFUN([CL_AS_NOEXECSTACK],[
dnl AC_REQUIRE([AC_PROG_CC]) GMP uses something else
AC_CACHE_CHECK([whether assembler supports --noexecstack option],
cl_cv_as_noexecstack, [dnl
  cat > conftest.c <<EOF
void foo() {}
EOF
  if AC_TRY_COMMAND([${CC} $CFLAGS $CPPFLAGS
                     -S -o conftest.s conftest.c >/dev/null]) \
     && grep .note.GNU-stack conftest.s >/dev/null \
     && AC_TRY_COMMAND([${CC} $CFLAGS $CPPFLAGS -Wa,--noexecstack
                       -c -o conftest.o conftest.s >/dev/null])
  then
    cl_cv_as_noexecstack=yes
  else
    cl_cv_as_noexecstack=no
  fi
  rm -f conftest*])
  if test "$cl_cv_as_noexecstack" = yes; then
    ASMFLAGS="$ASMFLAGS -Wa,--noexecstack"
  fi
  AC_SUBST(ASMFLAGS)
])

Note that --noexecstack gets eventually passed to the assembler, not -z noexecstack to the linker.

comment:29 Changed 4 years ago by fbissey

Ha, when I was looking at 7.0rc with Paul I only had gmp-6.1.0 and I still had a message about the executable stack from portage if I didn't add -Wl,-z,noexecstack to LDFLAGS. I moved to 6.1.1 on 21st of June. And indeed I don't need to add it anymore, it is added automatically:

libtool: link: x86_64-pc-linux-gnu-gcc -shared  -fPIC -DPIC  .libs/libecm_la-ecm.o .libs/libecm_la-ecm2.o .libs/libecm_la-pm1.o .libs/libecm_la-pp1.o .libs/libecm_la-getprime_r.o .libs/libecm_la-listz.o .libs/libecm_la-lucas.o .libs/libecm_la-stage2.o .libs/libecm_la-mpmod.o .libs/libecm_la-mul_lo.o .libs/libecm_la-polyeval.o .libs/libecm_la-median.o .libs/libecm_la-schoen_strass.o .libs/libecm_la-ks-multiply.o .libs/libecm_la-rho.o .libs/libecm_la-bestd.o .libs/libecm_la-auxlib.o .libs/libecm_la-random.o .libs/libecm_la-factor.o .libs/libecm_la-sp.o .libs/libecm_la-spv.o .libs/libecm_la-spm.o .libs/libecm_la-mpzspm.o .libs/libecm_la-mpzspv.o .libs/libecm_la-ntt_gfp.o .libs/libecm_la-ecm_ntt.o .libs/libecm_la-pm1fs2.o .libs/libecm_la-sets_long.o .libs/libecm_la-auxarith.o .libs/libecm_la-batch.o .libs/libecm_la-parametrizations.o .libs/libecm_la-cudawrapper.o aprtcle/.libs/libecm_la-mpz_aprcl.o  -Wl,--whole-archive ./x86_64/.libs/libmulredc.a -Wl,--no-whole-archive  -lgmp -lrt -lm  -g -O1 -march=native -ggdb -g -Wl,-znoexecstack -Wl,-O1 -Wl,--as-needed -O1 -march=native -ggdb   -Wl,-soname -Wl,libecm.so.1 -o .libs/libecm.so.1.0.0

comment:30 Changed 4 years ago by leif

I was just about to note that GMP-ECM consistently uses CCAS (which defaults to CC) and CCASFLAGS (which default to CFLAGS)...

comment:31 follow-up: Changed 4 years ago by fbissey

In any case sage's default is mpir so we should pass it in the default case. Passing it with gmp-6.1.1 doesn't really hurt.

comment:32 Changed 4 years ago by leif

Oh, GMP-ECM just harcodes it into Makefile.am:

libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g -Wl,-znoexecstack

(Executables are not linked with that option, but link to the above library, and presumably don't contain any other asm modules.)

And some linkers require a space IIRC...

comment:33 in reply to: ↑ 31 Changed 4 years ago by leif

Replying to fbissey:

In any case sage's default is mpir so we should pass it in the default case. Passing it with gmp-6.1.1 doesn't really hurt.

? The option doesn't originate from GMP (see my last comment).

comment:34 follow-up: Changed 4 years ago by fbissey

collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.

comment:35 in reply to: ↑ 34 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.

Well, until some linker bails out on that flag... Or is libtool smart enough to remove it in case it isn't supported?

comment:36 Changed 4 years ago by leif

  • Work issues set to Fix doctests and pexpect interface to ecm

comment:37 in reply to: ↑ 35 Changed 4 years ago by fbissey

Replying to leif:

Replying to fbissey:

collision. I posted before you. I guess it must have steamed from me communicating my concern to Paul. It definitely wasn't there in 7.0rc and I don't think it was in 7.0 either. One less thing to worry about.

Well, until some linker bails out on that flag... Or is libtool smart enough to remove it in case it isn't supported?

Point. Most linkers discard unrecognised option without side effect, the problem is when it can be interpreted in a completely different way by the linker (yes I have a "funny" example of that - it involves AIX's linker). It would be better if the flag was tested for in configure.

comment:38 Changed 4 years ago by leif

Did you already give it a try on MacOS X (recently)?

comment:39 Changed 4 years ago by fbissey

No, and it will have to wait until morning now because it is my bed time. And I don't think upstream does regular check on OS X.

comment:40 Changed 4 years ago by leif

What's weird is that despite

  --enable-shared[=PKGS]  build shared libraries [default=no]
  --enable-static[=PKGS]  build static libraries [default=yes]

and

...
checking whether the gcc-6.1 linker (ld -m elf_x86_64) supports shared libraries... yes
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... no
checking whether to build static libraries... yes
...

still a shared library gets built.

comment:41 follow-up: Changed 4 years ago by leif

Ok, at least within Sage, only a static library gets installed.

comment:42 in reply to: ↑ 41 ; follow-up: Changed 4 years ago by fbissey

Replying to leif:

Ok, at least within Sage, only a static library gets installed.

I personally don't think it is a good idea. Yes that's supposed to be better performance but if your going to mix it python module it will probably have to be compiled with pic which will remove part of your performance gain. Unless you have specialised hardware (embedded, BlueGene?) the benefits are marginal in my opinion.

comment:43 in reply to: ↑ 42 Changed 4 years ago by leif

Replying to fbissey:

Replying to leif:

Ok, at least within Sage, only a static library gets installed.

[...] if your going to mix it python module it will probably have to be compiled with pic

We of course enforce that in spkg-install.

which will remove part of your performance gain.

Not necessarily, depends on the processor. You need relative addressing, but not indirect function calls, and most symbols will get resolved at .so build time, not when loading the shared library.

Unless you have specialised hardware (embedded, BlueGene?) the benefits are marginal in my opinion.

Hmmm, but having a static library included in the Python extension module (which of course is a shared library) prevents us from further headaches, too, IMHO. :-)

comment:44 Changed 4 years ago by fbissey

OS X doesn't like this thing much

/bin/sh ./libtool  --tag=CC   --mode=link gcc  -g -march=native -g -O3  -fPIC  -version-info 1:0:0 -g -Wl,-znoexecstack  -L/Users/fbissey/build/sage-7.2.beta5/local/lib -Wl,-rpath,/Users/fbissey/build/sage-7.2.beta5/local/lib   -o libecm.la -rpath /Users/fbissey/build/sage-7.2.beta5/local/lib libecm_la-ecm.lo libecm_la-ecm2.lo libecm_la-pm1.lo libecm_la-pp1.lo libecm_la-getprime_r.lo libecm_la-listz.lo libecm_la-lucas.lo libecm_la-stage2.lo libecm_la-mpmod.lo libecm_la-mul_lo.lo libecm_la-polyeval.lo libecm_la-median.lo libecm_la-schoen_strass.lo libecm_la-ks-multiply.lo libecm_la-rho.lo libecm_la-bestd.lo libecm_la-auxlib.lo libecm_la-random.lo libecm_la-factor.lo libecm_la-sp.lo libecm_la-spv.lo libecm_la-spm.lo libecm_la-mpzspm.lo libecm_la-mpzspv.lo libecm_la-ntt_gfp.lo libecm_la-ecm_ntt.lo libecm_la-pm1fs2.lo libecm_la-sets_long.lo libecm_la-auxarith.lo libecm_la-batch.lo libecm_la-parametrizations.lo libecm_la-cudawrapper.lo aprtcle/libecm_la-mpz_aprcl.lo   ./x86_64/libmulredc.la  /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a -lm -lm -lm -lm -lm  

*** Warning: Linking the shared library libecm.la against the
*** static library /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a is not portable!
libtool: link: (cd .libs/libecm.lax/libmulredc.a && ar x "/Users/fbissey/build/sage-7.2.beta5/local/var/tmp/sage/build/ecm-7.0.3/src/./x86_64/.libs/libmulredc.a")
libtool: link: ar cru .libs/libecm.a /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a  libecm_la-ecm.o libecm_la-ecm2.o libecm_la-pm1.o libecm_la-pp1.o libecm_la-getprime_r.o libecm_la-listz.o libecm_la-lucas.o libecm_la-stage2.o libecm_la-mpmod.o libecm_la-mul_lo.o libecm_la-polyeval.o libecm_la-median.o libecm_la-schoen_strass.o libecm_la-ks-multiply.o libecm_la-rho.o libecm_la-bestd.o libecm_la-auxlib.o libecm_la-random.o libecm_la-factor.o libecm_la-sp.o libecm_la-spv.o libecm_la-spm.o libecm_la-mpzspm.o libecm_la-mpzspv.o libecm_la-ntt_gfp.o libecm_la-ecm_ntt.o libecm_la-pm1fs2.o libecm_la-sets_long.o libecm_la-auxarith.o libecm_la-batch.o libecm_la-parametrizations.o libecm_la-cudawrapper.o aprtcle/libecm_la-mpz_aprcl.o  .libs/libecm.lax/libmulredc.a/mulredc1.o .libs/libecm.lax/libmulredc.a/mulredc10.o .libs/libecm.lax/libmulredc.a/mulredc11.o .libs/libecm.lax/libmulredc.a/mulredc12.o .libs/libecm.lax/libmulredc.a/mulredc13.o .libs/libecm.lax/libmulredc.a/mulredc14.o .libs/libecm.lax/libmulredc.a/mulredc15.o .libs/libecm.lax/libmulredc.a/mulredc16.o .libs/libecm.lax/libmulredc.a/mulredc17.o .libs/libecm.lax/libmulredc.a/mulredc18.o .libs/libecm.lax/libmulredc.a/mulredc19.o .libs/libecm.lax/libmulredc.a/mulredc1_10.o .libs/libecm.lax/libmulredc.a/mulredc1_11.o .libs/libecm.lax/libmulredc.a/mulredc1_12.o .libs/libecm.lax/libmulredc.a/mulredc1_13.o .libs/libecm.lax/libmulredc.a/mulredc1_14.o .libs/libecm.lax/libmulredc.a/mulredc1_15.o .libs/libecm.lax/libmulredc.a/mulredc1_16.o .libs/libecm.lax/libmulredc.a/mulredc1_17.o .libs/libecm.lax/libmulredc.a/mulredc1_18.o .libs/libecm.lax/libmulredc.a/mulredc1_19.o .libs/libecm.lax/libmulredc.a/mulredc1_2.o .libs/libecm.lax/libmulredc.a/mulredc1_20.o .libs/libecm.lax/libmulredc.a/mulredc1_3.o .libs/libecm.lax/libmulredc.a/mulredc1_4.o .libs/libecm.lax/libmulredc.a/mulredc1_5.o .libs/libecm.lax/libmulredc.a/mulredc1_6.o .libs/libecm.lax/libmulredc.a/mulredc1_7.o .libs/libecm.lax/libmulredc.a/mulredc1_8.o .libs/libecm.lax/libmulredc.a/mulredc1_9.o .libs/libecm.lax/libmulredc.a/mulredc2.o .libs/libecm.lax/libmulredc.a/mulredc20.o .libs/libecm.lax/libmulredc.a/mulredc3.o .libs/libecm.lax/libmulredc.a/mulredc4.o .libs/libecm.lax/libmulredc.a/mulredc5.o .libs/libecm.lax/libmulredc.a/mulredc6.o .libs/libecm.lax/libmulredc.a/mulredc7.o .libs/libecm.lax/libmulredc.a/mulredc8.o .libs/libecm.lax/libmulredc.a/mulredc9.o 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: .libs/libecm.a(libecm_la-cudawrapper.o) has no symbols
libtool: link: ranlib .libs/libecm.a
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: .libs/libecm.a(libecm_la-cudawrapper.o) has no symbols
libtool: link: rm -fr .libs/libecm.lax
libtool: link: ( cd ".libs" && rm -f "libecm.la" && ln -s "../libecm.la" "libecm.la" )
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-auxi.o -MD -MP -MF .deps/ecm-auxi.Tpo -c -o ecm-auxi.o `test -f 'auxi.c' || echo './'`auxi.c
mv -f .deps/ecm-auxi.Tpo .deps/ecm-auxi.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-b1_ainc.o -MD -MP -MF .deps/ecm-b1_ainc.Tpo -c -o ecm-b1_ainc.o `test -f 'b1_ainc.c' || echo './'`b1_ainc.c
mv -f .deps/ecm-b1_ainc.Tpo .deps/ecm-b1_ainc.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-candi.o -MD -MP -MF .deps/ecm-candi.Tpo -c -o ecm-candi.o `test -f 'candi.c' || echo './'`candi.c
mv -f .deps/ecm-candi.Tpo .deps/ecm-candi.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-eval.o -MD -MP -MF .deps/ecm-eval.Tpo -c -o ecm-eval.o `test -f 'eval.c' || echo './'`eval.c
mv -f .deps/ecm-eval.Tpo .deps/ecm-eval.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-main.o -MD -MP -MF .deps/ecm-main.Tpo -c -o ecm-main.o `test -f 'main.c' || echo './'`main.c
mv -f .deps/ecm-main.Tpo .deps/ecm-main.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-resume.o -MD -MP -MF .deps/ecm-resume.Tpo -c -o ecm-resume.o `test -f 'resume.c' || echo './'`resume.c
mv -f .deps/ecm-resume.Tpo .deps/ecm-resume.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-getprime_r.o -MD -MP -MF .deps/ecm-getprime_r.Tpo -c -o ecm-getprime_r.o `test -f 'getprime_r.c' || echo './'`getprime_r.c
mv -f .deps/ecm-getprime_r.Tpo .deps/ecm-getprime_r.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT aprtcle/ecm-mpz_aprcl.o -MD -MP -MF aprtcle/.deps/ecm-mpz_aprcl.Tpo -c -o aprtcle/ecm-mpz_aprcl.o `test -f 'aprtcle/mpz_aprcl.c' || echo './'`aprtcle/mpz_aprcl.c
mv -f aprtcle/.deps/ecm-mpz_aprcl.Tpo aprtcle/.deps/ecm-mpz_aprcl.Po
gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM -I/Users/fbissey/build/sage-7.2.beta5/local/include -I/Users/fbissey/build/sage-7.2.beta5/local/include  -g -march=native -g -O3  -fPIC -MT ecm-memusage.o -MD -MP -MF .deps/ecm-memusage.Tpo -c -o ecm-memusage.o `test -f 'memusage.c' || echo './'`memusage.c
mv -f .deps/ecm-memusage.Tpo .deps/ecm-memusage.Po
/bin/sh ./libtool  --tag=CC   --mode=link gcc  -g -march=native -g -O3  -fPIC  -L/Users/fbissey/build/sage-7.2.beta5/local/lib -Wl,-rpath,/Users/fbissey/build/sage-7.2.beta5/local/lib   -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o libecm.la /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a -lm -lm -lm -lm -lm  
libtool: link: gcc -g -march=native -g -O3 -fPIC -Wl,-rpath -Wl,/Users/fbissey/build/sage-7.2.beta5/local/lib -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o  -L/Users/fbissey/build/sage-7.2.beta5/local/lib ./.libs/libecm.a /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a -lm
ld: warning: ignoring file ./.libs/libecm.a, file was built for archive which is not the architecture being linked (x86_64): ./.libs/libecm.a
Undefined symbols for architecture x86_64:
  "___ecm_cputime", referenced from:
      _main in ecm-main.o
  "_ecm_clear", referenced from:
      _main in ecm-main.o
  "_ecm_factor", referenced from:
      _main in ecm-main.o
  "_ecm_init", referenced from:
      _main in ecm-main.o
  "_ecm_version", referenced from:
      _main in ecm-main.o
ld: symbol(s) not found for architecture x86_64
collect2: error: ld returned 1 exit status
make[2]: *** [ecm] Error 1
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
Error building GMP-ECM.

It is not obvious that znoexecstack is at fault but I cannot rule it out.

I am otherwise in the camp that says dynamic linking is actually the lesser trouble :)

comment:45 Changed 4 years ago by leif

P.S.: Actually libgmp.a should get linked into the Python extension module as well, but that's currently not the case.

comment:46 follow-up: Changed 4 years ago by fbissey

Interesting znoexecstack is not a problem building static - it is probably just ignored but build shared with ECM_CONFIGURE="--enable-shared --disable-static

libtool: link: gcc -dynamiclib -Wl,-undefined -Wl,dynamic_lookup -o .libs/libecm.1.dylib  .libs/libecm_la-ecm.o .libs/libecm_la-ecm2.o .libs/libecm_la-pm1.o .libs/libecm_la-pp1.o .libs/libecm_la-getprime_r.o .libs/libecm_la-listz.o .libs/libecm_la-lucas.o .libs/libecm_la-stage2.o .libs/libecm_la-mpmod.o .libs/libecm_la-mul_lo.o .libs/libecm_la-polyeval.o .libs/libecm_la-median.o .libs/libecm_la-schoen_strass.o .libs/libecm_la-ks-multiply.o .libs/libecm_la-rho.o .libs/libecm_la-bestd.o .libs/libecm_la-auxlib.o .libs/libecm_la-random.o .libs/libecm_la-factor.o .libs/libecm_la-sp.o .libs/libecm_la-spv.o .libs/libecm_la-spm.o .libs/libecm_la-mpzspm.o .libs/libecm_la-mpzspv.o .libs/libecm_la-ntt_gfp.o .libs/libecm_la-ecm_ntt.o .libs/libecm_la-pm1fs2.o .libs/libecm_la-sets_long.o .libs/libecm_la-auxarith.o .libs/libecm_la-batch.o .libs/libecm_la-parametrizations.o .libs/libecm_la-cudawrapper.o aprtcle/.libs/libecm_la-mpz_aprcl.o   -Wl,-force_load,./x86_64/.libs/libmulredc.a  -L/Users/fbissey/build/sage-7.2.beta5/local/lib -lgmp -lm  -g -march=native -g -O3 -g -Wl,-znoexecstack -Wl,-rpath -Wl,/Users/fbissey/build/sage-7.2.beta5/local/lib   -install_name  /Users/fbissey/build/sage-7.2.beta5/local/lib/libecm.1.dylib -compatibility_version 2 -current_version 2.0 -Wl,-single_module
ld: unknown option: -znoexecstack
collect2: error: ld returned 1 exit status

And that definitely kills the linking. So we will need to remove it from upstream sources.

Yes once you link static stuff you need to provide all the bits (dependencies) since static libraries are dumb objects.

Last edited 4 years ago by fbissey (previous) (diff)

comment:47 follow-up: Changed 4 years ago by leif

Presumably unrelated, but 7.2.beta5?

What "architecture" in Apple speak do the object files in libecm.a have?

comment:48 in reply to: ↑ 47 Changed 4 years ago by fbissey

Replying to leif:

Presumably unrelated, but 7.2.beta5?

That's just the tarball I initially downloaded, but I pulled the latest develop and then your branch. I fully rebuild not so long ago.

What "architecture" in Apple speak do the object files in libecm.a have?

Good question, I'll see if I can figure it out.

comment:49 in reply to: ↑ 46 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

Interesting znoexecstack is not a problem building static - it is probably just ignored

It's not used at all when building static stuff, or more precisely, only when building libecm.la.


but build shared with ECM_CONFIGURE="--enable-shared --disable-static

ld: unknown option: -znoexecstack
collect2: error: ld returned 1 exit status

And that definitely kills the linking.

I told you so... ;-)


So we will need to remove it from upstream sources.

Well, patch a comma into it. (It's in Makefile.am and hence also in what's built from it.)

comment:50 in reply to: ↑ 49 ; follow-up: Changed 4 years ago by leif

Replying to leif:

Replying to fbissey:

ld: unknown option: -znoexecstack
collect2: error: ld returned 1 exit status

Well, patch a comma into it. (It's in Makefile.am and hence also in what's built from it.)

That is, in the source tarball only one instance in the top-level Makefile.in.

I wouldn't even use patch for that... ;-)

comment:51 Changed 4 years ago by leif

But I'm really in favour of submitting something upstream that in configure adds -Wa,--noexecstack to CCASFLAGS when supported (or maybe -Wl,-z,noexecstack to some *LDFLAGS).

Last edited 4 years ago by leif (previous) (diff)

comment:52 in reply to: ↑ 50 Changed 4 years ago by fbissey

Replying to leif:

Replying to leif:

Replying to fbissey:

ld: unknown option: -znoexecstack
collect2: error: ld returned 1 exit status

Well, patch a comma into it. (It's in Makefile.am and hence also in what's built from it.)

That is, in the source tarball only one instance in the top-level Makefile.in.

I wouldn't even use patch for that... ;-)

May be you wouldn't. But in terms of sage, I would do a spkg-src. You can only be that dirty in a big collaborative project of that kind for so long. That kind of stuff just doesn't scale and hurt you in the long term.

In my opinion the whole spkg-install is overdone. Just let the build system do its job unless you want to produce a binary.

+1 for a report upstream.

comment:53 Changed 4 years ago by leif

It's just always a headache to patch autotools files, and spkg-src is IMHO not a good idea unless you pull some svn version (as opposed to a released tarball), or really remove loads of unnecessary files in order to keep the tarball small.

comment:54 Changed 4 years ago by leif

(Sage's "build system" doesn't yet support upstream tarball URLs though, and the "design" of for example checksums.ini is ... -- can't say that here.)

comment:55 follow-up: Changed 4 years ago by fbissey

No comment. I am the sage-on-gentoo maintainer - anything I say on the matter is kind of political.

Back to our little problem on OS X. It looks like it is something done in spkg-install, going to a sage shell and the build location I just do

make distclean
./configure # by default only build static
make

is successful and

(sage-sh) fbissey@Mirage:src$ make check
Making check in x86_64
make[1]: Nothing to be done for `check'.
make  ecm \
	  test.pp1 test.pm1 test.ecm 
make[2]: `ecm' is up to date.
make[2]: Nothing to be done for `test.pp1'.
make[2]: Nothing to be done for `test.pm1'.
make[2]: Nothing to be done for `test.ecm'.
make  check-TESTS
PASS: test.pp1
PASS: test.pm1
PASS: test.ecm
============================================================================
Testsuite summary for ecm 7.0.3
============================================================================
# TOTAL: 3
# PASS:  3
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

so it is definitely the configuration/environment from spkg-install, the configuration picked up sage's gcc as the compiler and sage's mpir as gmp without a hitch and assembly code has been used.

comment:56 Changed 4 years ago by git

  • Commit changed from 0affd290f491cf624f15fec080f3b349bd07477c to 6741ee0e6e3d3570157fb453c54bcde586bffea8

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

6741ee0GMP-ECM: #20385: Add temporary patch for '-Wl,znoexecstack' [sic] (Makefile.{am,in})

comment:57 Changed 4 years ago by leif

  • ecm-7.0.3/Makefile.am

    #20385:  Fix currently hardcoded '-Wl,znoexecstack' because
    there's a space missing.  (E.g. Apple's linker bails out on
    '-znoexecstack', while GNU ld just treats it as if it was
    '-z noexecstack', the correct form.)
    
    This is an ugly, temporary fix until we submit something
    better upstream.
    
    First of all, 'configure' should check whether this is / which
    options are supported and add them based on that, and it's
    presumably better to just put '-Wa,--noexecstack' into
    CCASFLAGS when the assembler supports that option.
    
    
    diff -Naur ecm-7.0.3.orig/Makefile.am ecm-7.0.3/Makefile.am
     
    5050# www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
    5151# If any interfaces have been added, removed, or changed since the last
    5252# update, increment current, and set revision to 0.
    53 libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g -Wl,-znoexecstack
     53libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g -Wl,-z,noexecstack
    5454libecm_la_LIBADD = $(MULREDCLIBRARY)
    5555if WANT_GPU
    5656  libecm_la_SOURCES += cudakernel.cu
  • ecm-7.0.3/Makefile.in

    diff -Naur ecm-7.0.3.orig/Makefile.in ecm-7.0.3/Makefile.in
     
    715715# If any interfaces have been added, removed, or changed since the last
    716716# update, increment current, and set revision to 0.
    717717libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g \
    718         -Wl,-znoexecstack $(am__append_5)
     718        -Wl,-z,noexecstack $(am__append_5)
    719719libecm_la_LIBADD = $(MULREDCLIBRARY) $(am__append_4) $(GMPLIB)
    720720@WANT_GPU_TRUE@ecm_LDFLAGS = $(CUDARPATH)
    721721@WITH_GWNUM_TRUE@ecm_LDFLAGS = $(AM_LDFLAGS) -Wl,gwdata.ld

New commits:

6741ee0GMP-ECM: #20385: Add temporary patch for '-Wl,znoexecstack' [sic] (Makefile.{am,in})

comment:58 follow-up: Changed 4 years ago by fbissey

With that I have

ld: unknown option: -z

now when enabling shared libraries on OS X. At least that's consistent with the man page.

comment:59 in reply to: ↑ 55 Changed 4 years ago by leif

Replying to fbissey:

Back to our little problem on OS X. It looks like it is something done in spkg-install, going to a sage shell and the build location I just do

make distclean
./configure # by default only build static
make

is successful [...] so it is definitely the configuration/environment from spkg-install, the configuration picked up sage's gcc as the compiler and sage's mpir as gmp without a hitch and assembly code has been used.

Well, post the part of the log where spkg-install prints all the relevant environment variables and configure options passed.

comment:60 in reply to: ↑ 58 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

With that I have

ld: unknown option: -z

now when enabling shared libraries on OS X. At least that's consistent with the man page.

What does gcc -c -Wa,--noexecstack foo.c give? (foo.c at your like, e.g. void foo() {}.)

(And probably also gcc -v -o foo ... without -c.)

comment:61 in reply to: ↑ 60 Changed 4 years ago by leif

Replying to leif:

(And probably also gcc -v -o foo ... without -c.)

Ahem, forget that.

comment:62 Changed 4 years ago by fbissey

clang: error: unsupported option '--noexecstack'

Not in the manual either.

comment:63 Changed 4 years ago by leif

And/or attach the log of the last failed build here.

Changed 4 years ago by fbissey

ecm log - bare spkg-install run

Changed 4 years ago by fbissey

config.log matching ecm.log

comment:64 Changed 4 years ago by fbissey

Done. So just plain ./sage -p ecm logs. No funky things in ECM_CONFIGURE.

comment:65 Changed 4 years ago by leif

  • Attachment ecm.log​ added

ecm log - bare spkg-install run

Interestingly, ld there didn't bail out on -z noexecstack, for whatever reason. (Did it at all get invoked?)

Otherwise I'm stuck at libtool weirdness and what Apple's ld considers "not x86_64 architecture".

Also, while we have

checking for gcc option to produce PIC... -fno-common -DPIC
checking if gcc PIC flag -fno-common -DPIC works... yes
checking if gcc static flag -static works... no

only -fPIC is used (which we add in spkg-install).

And libtool of courseTM keeps building .o and .lo files, and both foo.a and foo.la.

comment:66 follow-up: Changed 4 years ago by fbissey

Found the bit that cause problem --with-gmp=${SAGE_LOCAL} without it configure tells me

configure: Configuration:
configure: Build for host type x86_64-apple-darwin15.6.0
configure: CC=gcc, CFLAGS=-g -O2
configure: Linking GMP with -lgmp

and the build succeeds. With it

configure: Configuration:
configure: Build for host type x86_64-apple-darwin15.6.0
configure: CC=gcc, CFLAGS=-g -O2
configure: Linking GMP with /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a

and the build fails....

comment:67 Changed 4 years ago by fbissey

Rebuilding develop branch from scratch to make sure mpir is as sane as it can be.

comment:68 in reply to: ↑ 66 Changed 4 years ago by leif

Replying to fbissey:

Found the bit that cause problem --with-gmp=${SAGE_LOCAL} without it configure tells me

configure: Configuration:
configure: Build for host type x86_64-apple-darwin15.6.0
configure: CC=gcc, CFLAGS=-g -O2
configure: Linking GMP with -lgmp

I thought of that, but you told me "the configuration picked up sage's gcc as the compiler and sage's mpir as gmp without a hitch" and I was wondering whether that's due to Sage GCC's configuration.


But before my internet connection went down the drain 8/ I was about to post:


And then I'd need a log from your successful "manual" build (make distclean && ./configure && make) from within a Sage subshell to compare with.

OTOH the only (major) difference -- regarding the environment when configure is invoked -- should be -fPIC in CFLAGS, as ECM picks up MPIR's when CFLAGS are empty / not set (which is the case in a Sage subshell unless you've set them yourself).

Not sure how libtool reacts to that, or whether there's anything in configure that's influenced by that.


Now clearly the difference seems to (also) be that you don't have a static libgmp installed system-wide.

comment:69 follow-up: Changed 4 years ago by fbissey

I don't even have a dynamic one.

The environment passes -L${SAGE_LOCAL}/lib and -Wl,-rpath=${SAGE_LOCAL}/lib, further the configure script identifies gmp as being in fact mpir in both case. So the -lgmp picked can only be from ${SAGE_LOCAL}/lib.

Even further when outside of the sage shell configure bails out because it cannot find any gmp. So the only gmp available is in sage's install.

On another front I separately tried to add -fPIC to CFLAGS for configuration and that wasn't a problem. Just adding -fPIC lead to a successful compilation.

libgmp.a or the process of using libgmp.a wholesale is causing trouble. Using libgmp.a that way tries to fold the gmp archive inside libecm.a (so you don't need to link with libgmp.a as it is already inside), apparently it causes trouble on OS X. Possibly something in mpir rather than ecm in my opinion.

comment:70 in reply to: ↑ 69 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

libgmp.a or the process of using libgmp.a wholesale is causing trouble. Using libgmp.a that way tries to fold the gmp archive inside libecm.a (so you don't need to link with libgmp.a as it is already inside), apparently it causes trouble on OS X. Possibly something in mpir rather than ecm in my opinion.

It shouldn't. You cannot "fold" a static library into another, you can include object files from one into another, or when you build a shared library, include object files into the former (as needed -- symbols get resolved and the object files from which they originate get included, all ending up in the same single text segment etc.).


Wondering whether we build MPIR's/GMP's static library also with PIC. (Btw., ECM also has --with-pic.)


My guess is rather that it's ECM's fault.

Last edited 4 years ago by leif (previous) (diff)

comment:71 in reply to: ↑ 70 Changed 4 years ago by leif

Replying to leif:

Wondering whether we build MPIR's/GMP's static library also with PIC.

Looks like it (MPIR, not explicitly we) does (-fPIC -DPIC).

comment:72 Changed 4 years ago by leif

Note that in

libtool: link: gcc -g -march=native -g -O3 -fPIC
    -Wl,-rpath -Wl,/Users/fbissey/build/sage-7.2.beta5/local/lib 
    -o ecm
    ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o
    ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o
    -L/Users/fbissey/build/sage-7.2.beta5/local/lib ./.libs/libecm.a
    /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a -lm
ld: warning: ignoring file ./.libs/libecm.a, file was built for archive which is not the architecture being linked (x86_64): ./.libs/libecm.a

the linker only complains about libecm.a, but not libgmp.a.

comment:73 Changed 4 years ago by leif

But you're right, ECM actually adds libgmp.a to libecm.a:

libtool: link: ar cru .libs/libecm.a /Users/fbissey/build/sage-7.2.beta5/local/lib/libgmp.a  libecm_la-ecm.o libecm_la-ecm2.o libecm_la-pm1.o libecm_la-pp1.o libecm_la-getprime_r.o libecm_la-listz.o libecm_la-lucas.o libecm_la-stage2.o libecm_la-mpmod.o libecm_la-mul_lo.o libecm_la-polyeval.o libecm_la-median.o libecm_la-schoen_strass.o libecm_la-ks-multiply.o libecm_la-rho.o libecm_la-bestd.o libecm_la-auxlib.o libecm_la-random.o libecm_la-factor.o libecm_la-sp.o libecm_la-spv.o libecm_la-spm.o libecm_la-mpzspm.o libecm_la-mpzspv.o libecm_la-ntt_gfp.o libecm_la-ecm_ntt.o libecm_la-pm1fs2.o libecm_la-sets_long.o libecm_la-auxarith.o libecm_la-batch.o libecm_la-parametrizations.o libecm_la-cudawrapper.o aprtcle/libecm_la-mpz_aprcl.o  .libs/libecm.lax/libmulredc.a/mulredc1.o .libs/libecm.lax/libmulredc.a/mulredc10.o .libs/libecm.lax/libmulredc.a/mulredc11.o .libs/libecm.lax/libmulredc.a/mulredc12.o .libs/libecm.lax/libmulredc.a/mulredc13.o .libs/libecm.lax/libmulredc.a/mulredc14.o .libs/libecm.lax/libmulredc.a/mulredc15.o .libs/libecm.lax/libmulredc.a/mulredc16.o .libs/libecm.lax/libmulredc.a/mulredc17.o .libs/libecm.lax/libmulredc.a/mulredc18.o .libs/libecm.lax/libmulredc.a/mulredc19.o .libs/libecm.lax/libmulredc.a/mulredc1_10.o .libs/libecm.lax/libmulredc.a/mulredc1_11.o .libs/libecm.lax/libmulredc.a/mulredc1_12.o .libs/libecm.lax/libmulredc.a/mulredc1_13.o .libs/libecm.lax/libmulredc.a/mulredc1_14.o .libs/libecm.lax/libmulredc.a/mulredc1_15.o .libs/libecm.lax/libmulredc.a/mulredc1_16.o .libs/libecm.lax/libmulredc.a/mulredc1_17.o .libs/libecm.lax/libmulredc.a/mulredc1_18.o .libs/libecm.lax/libmulredc.a/mulredc1_19.o .libs/libecm.lax/libmulredc.a/mulredc1_2.o .libs/libecm.lax/libmulredc.a/mulredc1_20.o .libs/libecm.lax/libmulredc.a/mulredc1_3.o .libs/libecm.lax/libmulredc.a/mulredc1_4.o .libs/libecm.lax/libmulredc.a/mulredc1_5.o .libs/libecm.lax/libmulredc.a/mulredc1_6.o .libs/libecm.lax/libmulredc.a/mulredc1_7.o .libs/libecm.lax/libmulredc.a/mulredc1_8.o .libs/libecm.lax/libmulredc.a/mulredc1_9.o .libs/libecm.lax/libmulredc.a/mulredc2.o .libs/libecm.lax/libmulredc.a/mulredc20.o .libs/libecm.lax/libmulredc.a/mulredc3.o .libs/libecm.lax/libmulredc.a/mulredc4.o .libs/libecm.lax/libmulredc.a/mulredc5.o .libs/libecm.lax/libmulredc.a/mulredc6.o .libs/libecm.lax/libmulredc.a/mulredc7.o .libs/libecm.lax/libmulredc.a/mulredc8.o .libs/libecm.lax/libmulredc.a/mulredc9.o 

and that's presumably the entry Apple's ld complains about (and instead of ignoring the entry, it drops the whole archive, libecm.a).

comment:74 Changed 4 years ago by leif

I guess this is the culprit (in Makefile.am, line 61):

libecm_la_LIBADD += $(GMPLIB)

(In Makefile.in, on line 719.)


New tentative patch:

  • ecm-7.0.3/Makefile.am

    #20385:  Don't put $(GMPLIB) (=libgmp.a) into libecm.a, as Darwin's
    linker bails out on nested archives.  Probably a weird idea anyway,
    or some libtool bug, as it's presumably supposed to add *the contents*
    (i.e., the object files) of libgmp.a into libecm.a.
    
    
    diff -Naur ecm-7.0.3.orig/Makefile.am ecm-7.0.3/Makefile.am
     
    5858  libecm_la_LDFLAGS += $(CUDALDFLAGS)
    5959  ecm_LDFLAGS = $(CUDARPATH)
    6060endif
    61 libecm_la_LIBADD += $(GMPLIB)
     61# libecm_la_LIBADD += $(GMPLIB)
    6262
    6363bin_PROGRAMS = ecm
    6464noinst_PROGRAMS = tune ecmfactor bench_mulredc aprcl
  • ecm-7.0.3/Makefile.in

    diff -Naur ecm-7.0.3.orig/Makefile.in ecm-7.0.3/Makefile.in
     
    716716# update, increment current, and set revision to 0.
    717717libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g \
    718718        -Wl,-znoexecstack $(am__append_5)
    719 libecm_la_LIBADD = $(MULREDCLIBRARY) $(am__append_4) $(GMPLIB)
     719libecm_la_LIBADD = $(MULREDCLIBRARY) $(am__append_4)
    720720@WANT_GPU_TRUE@ecm_LDFLAGS = $(CUDARPATH)
    721721@WITH_GWNUM_TRUE@ecm_LDFLAGS = $(AM_LDFLAGS) -Wl,gwdata.ld
    722722# Most binaries want to link libecm.la, and the ones which don't will

Note that this new patch gets applied before the other one, so I had to change the latter as well since they unfortunately overlap. (But the first I added will get dropped later anyway, if Darwin's linker doesn't understand -z ... and also errors out on it.)

comment:75 Changed 4 years ago by leif

I'll push the changes in a couple of minutes...

comment:76 Changed 4 years ago by fbissey

Yes, removing that bit looks right. After all it is already added to LDFLAGS by configure. For the record I successfully installed ecm-7.0.3 on OS X with the following change

  • build/pkgs/ecm/spkg-install

    diff --git a/build/pkgs/ecm/spkg-install b/build/pkgs/ecm/spkg-install
    index fc49b3d..7c1e42a 100755
    a b else 
    237237fi
    238238echo "  --prefix=\"$SAGE_LOCAL\""
    239239echo "  --libdir=\"$SAGE_LOCAL/lib\""
    240 echo "  --with-gmp=\"$SAGE_LOCAL\""
    241240for opt in $ECM_CONFIGURE; do
    242241    echo "  $opt"
    243242done
    fi 
    249248echo
    250249
    251250./configure --prefix="$SAGE_LOCAL" --libdir="$SAGE_LOCAL/lib" \
    252             --with-gmp="$SAGE_LOCAL" \
    253251            $ECM_CONFIGURE
    254252if [ $? -ne 0 ]; then
    255253    echo >&2 "Error configuring GMP-ECM."

Just doing that removes libgmp.a from the ar call

libtool: link: (cd .libs/libecm.lax/libmulredc.a && ar x "/Users/fbissey/build/sage-7.2.beta5/local/var/tmp/sage/build/ecm-7.0.3/src/./x86_64/.libs/libmulredc.a")
libtool: link: ar cru .libs/libecm.a  libecm_la-ecm.o libecm_la-ecm2.o libecm_la-pm1.o libecm_la-pp1.o libecm_la-getprime_r.o libecm_la-listz.o libecm_la-lucas.o libecm_la-stage2.o libecm_la-mpmod.o libecm_la-mul_lo.o libecm_la-polyeval.o libecm_la-median.o libecm_la-schoen_strass.o libecm_la-ks-multiply.o libecm_la-rho.o libecm_la-bestd.o libecm_la-auxlib.o libecm_la-random.o libecm_la-factor.o libecm_la-sp.o libecm_la-spv.o libecm_la-spm.o libecm_la-mpzspm.o libecm_la-mpzspv.o libecm_la-ntt_gfp.o libecm_la-ecm_ntt.o libecm_la-pm1fs2.o libecm_la-sets_long.o libecm_la-auxarith.o libecm_la-batch.o libecm_la-parametrizations.o libecm_la-cudawrapper.o aprtcle/libecm_la-mpz_aprcl.o  .libs/libecm.lax/libmulredc.a/mulredc1.o .libs/libecm.lax/libmulredc.a/mulredc10.o .libs/libecm.lax/libmulredc.a/mulredc11.o .libs/libecm.lax/libmulredc.a/mulredc12.o .libs/libecm.lax/libmulredc.a/mulredc13.o .libs/libecm.lax/libmulredc.a/mulredc14.o .libs/libecm.lax/libmulredc.a/mulredc15.o .libs/libecm.lax/libmulredc.a/mulredc16.o .libs/libecm.lax/libmulredc.a/mulredc17.o .libs/libecm.lax/libmulredc.a/mulredc18.o .libs/libecm.lax/libmulredc.a/mulredc19.o .libs/libecm.lax/libmulredc.a/mulredc1_10.o .libs/libecm.lax/libmulredc.a/mulredc1_11.o .libs/libecm.lax/libmulredc.a/mulredc1_12.o .libs/libecm.lax/libmulredc.a/mulredc1_13.o .libs/libecm.lax/libmulredc.a/mulredc1_14.o .libs/libecm.lax/libmulredc.a/mulredc1_15.o .libs/libecm.lax/libmulredc.a/mulredc1_16.o .libs/libecm.lax/libmulredc.a/mulredc1_17.o .libs/libecm.lax/libmulredc.a/mulredc1_18.o .libs/libecm.lax/libmulredc.a/mulredc1_19.o .libs/libecm.lax/libmulredc.a/mulredc1_2.o .libs/libecm.lax/libmulredc.a/mulredc1_20.o .libs/libecm.lax/libmulredc.a/mulredc1_3.o .libs/libecm.lax/libmulredc.a/mulredc1_4.o .libs/libecm.lax/libmulredc.a/mulredc1_5.o .libs/libecm.lax/libmulredc.a/mulredc1_6.o .libs/libecm.lax/libmulredc.a/mulredc1_7.o .libs/libecm.lax/libmulredc.a/mulredc1_8.o .libs/libecm.lax/libmulredc.a/mulredc1_9.o .libs/libecm.lax/libmulredc.a/mulredc2.o .libs/libecm.lax/libmulredc.a/mulredc20.o .libs/libecm.lax/libmulredc.a/mulredc3.o .libs/libecm.lax/libmulredc.a/mulredc4.o .libs/libecm.lax/libmulredc.a/mulredc5.o .libs/libecm.lax/libmulredc.a/mulredc6.o .libs/libecm.lax/libmulredc.a/mulredc7.o .libs/libecm.lax/libmulredc.a/mulredc8.o .libs/libecm.lax/libmulredc.a/mulredc9.o 
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: .libs/libecm.a(libecm_la-cudawrapper.o) has no symbols
libtool: link: ranlib .libs/libecm.a

comment:77 Changed 4 years ago by git

  • Commit changed from 6741ee0e6e3d3570157fb453c54bcde586bffea8 to e115141455e83eb0213a83d4153f72d80a767e2d

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

e115141GMP-ECM: #20385: Add another patch avoiding nested archives and rebase the previous on it.

comment:78 follow-ups: Changed 4 years ago by leif

Pushed the new patch(es), and thereby deleted my comment I hadn't posted yet -- no idea what I had written... >8( Thanks, HTTPS!

... I seem to remember that I had speculated that ..._LIBADD = $(GMPLIB) would presumably have worked if $(GMPLIB) was libgmp.la, as libtool indeed unpacks $(MULREDCLIBRARY) and adds its contents to the libecm archive.


Still have to see what harm my patch does to a shared libecm; it mayget underlinked now.

comment:79 in reply to: ↑ 78 Changed 4 years ago by fbissey

Replying to leif:

Pushed the new patch(es), and thereby deleted my comment I hadn't posted yet -- no idea what I had written... >8( Thanks, HTTPS!

... I seem to remember that I had speculated that ..._LIBADD = $(GMPLIB) would presumably have worked if $(GMPLIB) was libgmp.la, as libtool indeed unpacks $(MULREDCLIBRARY) and adds its contents to the libecm archive.


Still have to see what harm my patch does to a shared libecm; it mayget underlinked now.

I wouldn't think so. Using libgmp.la means that it has to be present on the system. Relying on an installed, external to your project, .la file is extremely fragile. In any case I don't appear to have any lib{gmp,mpir}.la installed on OS X so it would fail - miserably.

comment:80 in reply to: ↑ 78 ; follow-up: Changed 4 years ago by leif

Replying to leif:

Still have to see what harm my patch does to a shared libecm; it may get underlinked now.

Exactly that happens.

comment:81 follow-up: Changed 4 years ago by leif

libecm_la_LDFLAGS += $(GMPLIB) to avoid underlinking?

comment:82 in reply to: ↑ 81 ; follow-up: Changed 4 years ago by leif

Replying to leif:

libecm_la_LDFLAGS += $(GMPLIB) to avoid underlinking?

Rather libecm_la_LDADD += $(GMPLIB) I think.

comment:83 in reply to: ↑ 80 Changed 4 years ago by leif

Replying to leif:

Replying to leif:

Still have to see what harm my patch does to a shared libecm; it may get underlinked now.

Exactly that happens.

But AFAICS that wouldn't be a regression w.r.t. GMP-ECM 6.4.4 at least...

comment:84 in reply to: ↑ 82 ; follow-up: Changed 4 years ago by leif

Replying to leif:

Replying to leif:

libecm_la_LDFLAGS += $(GMPLIB) to avoid underlinking?

Rather libecm_la_LDADD += $(GMPLIB) I think.

François, could you test this (adding the latter to Makefile.am)?

I don't have automake 1.15 installed...

comment:85 follow-up: Changed 4 years ago by fbissey

Just looked at ecm's configure.ac. I am not impressed but I guess this is geared towards static linking. However the proper way tp link statically is not add $path_to/libname.a, if you have gcc you pass -static.

I can test adding that stuff, give me a few minutes (i may suffer from interruptions from a 5 year old that needs to go to bed).

comment:86 in reply to: ↑ 85 Changed 4 years ago by leif

Replying to fbissey:

Just looked at ecm's configure.ac. I am not impressed but I guess this is geared towards static linking. However the proper way to link statically is not add $path_to/libname.a, if you have gcc you pass -static.

But that apparently doesn't work on Darwin, as in your log we had:

checking if gcc static flag -static works... no

Not sure what libtool magic offers.

Passing foo.a is certainly safer and more portable.

comment:87 follow-up: Changed 4 years ago by fbissey

One to bed. libtool as an -all-static mode for linking program that use only static libraries, not sure how it applies on OS X. But you cannot do that with libraries. You don't link static libraries, you just just ar+ranlib or equivalent. Folding one library in another is probably frowned on by libtool.

comment:88 in reply to: ↑ 84 Changed 4 years ago by fbissey

Replying to leif:

Replying to leif:

Replying to leif:

libecm_la_LDFLAGS += $(GMPLIB) to avoid underlinking?

Rather libecm_la_LDADD += $(GMPLIB) I think.

François, could you test this (adding the latter to Makefile.am)?

I don't have automake 1.15 installed...

Didn't really like it

***** automake *****
***** PWD: /scratch2/portage/sci-mathematics/gmp-ecm-7.0.3-r1/work/ecm-7.0.3
***** automake --add-missing --copy --force-missing

configure.ac:161: installing './compile'
configure.ac:8: installing './missing'
Makefile.am:61: error: libecm_la_LDADD must be set with '=' before using '+='
Makefile.am:61: error: use 'libecm_la_LIBADD', not 'libecm_la_LDADD'
Makefile.am: installing './depcomp'
Makefile.am:61: warning: variable 'libecm_la_LDADD' is defined but no program or
Makefile.am:61: library has 'libecm_la' as canonical name (possible typo)

Using = instead of += doesn't really help the second message

***** automake *****
***** PWD: /scratch2/portage/sci-mathematics/gmp-ecm-7.0.3-r1/work/ecm-7.0.3
***** automake --add-missing --copy --force-missing

configure.ac:161: installing './compile'
configure.ac:8: installing './missing'
Makefile.am:61: error: use 'libecm_la_LIBADD', not 'libecm_la_LDADD'
Makefile.am: installing './depcomp'
Makefile.am:61: warning: variable 'libecm_la_LDADD' is defined but no program or
Makefile.am:61: library has 'libecm_la' as canonical name (possible typo)

comment:89 Changed 4 years ago by leif

While I've now installed automake 1.15, it removes

runstatedir = @runstatedir@

from Makefile.in (without me having made any changes to Makefile.am)...

comment:90 follow-up: Changed 4 years ago by fbissey

That's the bit I don't like in configure.ac:

GMPLDFLAGS=""
if test -d "$with_gmp_lib"; then
  GMPLDFLAGS="-L$with_gmp_lib"
fi
GMPLIB="-lgmp"
if test "x$enable_shared" != xyes; then
  if test -r "$with_gmp_lib/libgmp.a"; then
    GMPLIB="$with_gmp_lib/libgmp.a"  <==================================
    dnl Don't need -L flag since we give full path to libgmp.a
    GMPLDFLAGS=""
  fi
fi
AC_SUBST([GMPLIB])
LDFLAGS="$LDFLAGS $GMPLDFLAGS"

comment:91 in reply to: ↑ 87 Changed 4 years ago by leif

Replying to fbissey:

You don't link static libraries, you just just ar+ranlib or equivalent. Folding one library in another is probably frowned on by libtool.

No, that's exactly what it does with $(MULREDCLIBRARY) (aka libmulredc.[l]a), and it doesn't nest the archive, but "flattens" both libraries / archives into one.

(And there's also ld -r ..., but merging all object files into one is even more crude and will not always work.)

comment:92 in reply to: ↑ 90 Changed 4 years ago by leif

Replying to fbissey:

That's the bit I don't like in configure.ac:

GMPLDFLAGS=""
if test -d "$with_gmp_lib"; then
  GMPLDFLAGS="-L$with_gmp_lib"
fi
GMPLIB="-lgmp"
if test "x$enable_shared" != xyes; then
  if test -r "$with_gmp_lib/libgmp.a"; then
    GMPLIB="$with_gmp_lib/libgmp.a"  <==================================
    dnl Don't need -L flag since we give full path to libgmp.a
    GMPLDFLAGS=""
  fi
fi
AC_SUBST([GMPLIB])
LDFLAGS="$LDFLAGS $GMPLDFLAGS"

Well add

  if test -r "$with_gmp_lib/libgmp.la"; then
    GMPLIB="$with_gmp_lib/libgmp.la" # That would have worked on Darwin as well.
  elif test -r "$with_gmp_lib/libgmp.a"; then
    GMPLIB="$with_gmp_lib/libgmp.a"
  ...

The meaning of --enable-shared isn't always clear.

Of course, you want a shared library (also) built, but that doesn't necessarily imply you want it itself to be dynamically linked (to libraries it depends on, no matter whether also built by the package or externally supplied).

I'd add an option --prefer-static[-gmp], say, in addition.

comment:93 Changed 4 years ago by leif

P.S.: IMHO the real bug in what you pointed out is that ECM allows to build static and dynamic libraries at the same time, so it should (if present) define and use both GMPLIB (static) and GMPDYLIB (say, shared).

comment:94 Changed 4 years ago by fbissey

I spent some time with the libtool mailing list and done some experiments. If you want to have a full static binary (program only), you will have to add -all-static to ${prog}_LDFLAGS. If you miss one of dependent library in static form: boom

/bin/sh ./libtool  --tag=CC   --mode=link x86_64-pc-linux-gnu-gcc  -g -O1 -march=native -pipe -ggdb -all-static -Wl,-O1 -Wl,--as-needed -O1 -march=native -pipe -ggdb  -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o libecm.la -lgmp -lrt -lm -lm -lm -lm -lm  
/bin/sh ./libtool  --tag=CC   --mode=link x86_64-pc-linux-gnu-gcc  -lpthread -O1 -march=native -pipe -ggdb  -Wl,-O1 -Wl,--as-needed -O1 -march=native -pipe -ggdb  -o ecmfactor ecmfactor-ecmfactor.o libecm.la -lgmp -lrt -lm -lm -lm -lm -lm  
libtool: link: x86_64-pc-linux-gnu-gcc -O1 -march=native -pipe -ggdb -Wl,-O1 -O1 -march=native -pipe -ggdb -o ecmfactor ecmfactor-ecmfactor.o  -lpthread -Wl,--as-needed ./.libs/libecm.a -lgmp -lrt -lm
libtool: link: x86_64-pc-linux-gnu-gcc -g -O1 -march=native -pipe -ggdb -static -Wl,-O1 -O1 -march=native -pipe -ggdb -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o  -Wl,--as-needed ./.libs/libecm.a -lgmp -lrt -lm
/usr/lib/gcc/x86_64-pc-linux-gnu/5.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: cannot find -lgmp
collect2: error: ld returned 1 exit status

In a normal scenario it is ok to build both static and dynamic at the same time. The dynamic library can be linked to the dependent libraries and the static one usually doesn't incorporate the dependent libraries (whatever you want to call the process).

The --enable-{shared,static} are concerning the building of libraries only. It doesn't say anything about programs which will be linked the default way unless you take steps.

You are right there is problem here when you try to build both static and dynamic libraries because the static one can use a deifferent recipe from normal if a static gmp is also available. At that point we have a break down.

The intent is probably to really have a mega-archive libecm.a including libgmp.a - while it is ok to incorporate internal libraies it is doubtful to do it with external ones. The best, sane, option I can offer is to compile the ecm executable all static if libgmp.a is available.

comment:95 Changed 4 years ago by leif

I think this is (almost) what we want:

  • ecm-7.0.3/Makefile.am

    #20385:  After having removed $(GMPLIB) from libecm_la_LIBADD,
    put it into libecm_la_LDFLAGS again, such that a dynamic library
    (if built) won't be underlinked.
    
    (I think this was ECM's intention.)
    
    
     
    5959  ecm_LDFLAGS = $(CUDARPATH)
    6060endif
    6161# libecm_la_LIBADD += $(GMPLIB)
     62libecm_la_LDFLAGS += $(GMPLIB)
    6263
    6364bin_PROGRAMS = ecm
    6465noinst_PROGRAMS = tune ecmfactor bench_mulredc aprcl
  • ecm-7.0.3/Makefile.in

     
    714712# www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
    715713# If any interfaces have been added, removed, or changed since the last
    716714# update, increment current, and set revision to 0.
     715# libecm_la_LIBADD += $(GMPLIB)
    717716libecm_la_LDFLAGS = $(LIBECM_LDFLAGS) -version-info 1:0:0 -g \
    718         -Wl,-z,noexecstack $(am__append_5)
     717        -Wl,-z,noexecstack $(am__append_5) $(GMPLIB)
    719718libecm_la_LIBADD = $(MULREDCLIBRARY) $(am__append_4)
    720719@WANT_GPU_TRUE@ecm_LDFLAGS = $(CUDARPATH)
    721720@WITH_GWNUM_TRUE@ecm_LDFLAGS = $(AM_LDFLAGS) -Wl,gwdata.ld

(Without modifying configure.ac [yet] at least.)

comment:96 follow-up: Changed 4 years ago by fbissey

In Makefile.am you want to change line 52 which already has a definition for libecm_la_LDFLAGS.

comment:97 Changed 4 years ago by leif

We should really ask upstream what they want(ed) or intend(ed), for example it IMHO makes sense to build statically linked programs (or incorporate libgmp.a) provided a static libgmp is available, unless --disable-static was given.

Building "everything" dynamically linked just because --enable-shared was (also) given IMHO doesn't make sense here.

(And I'd actually incorporate libgmp.a into libecm.so unless specified otherwise, which would require a new configure option.)

comment:98 in reply to: ↑ 96 ; follow-up: Changed 4 years ago by leif

Replying to fbissey:

In Makefile.am you want to change line 52 which already has a definition for libecm_la_LDFLAGS.

Oooops, forgot a plus, but for whatever reason, it worked for me (with version info etc.):

/bin/bash ./libtool  --tag=CC   --mode=link gcc-6.1  -g -march=native -g -O3   -version-info 1:0:0 -g -Wl,-z,noexecstack  -lgmp -L/data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib -Wl,-rpath,/data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib  -L/data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib -o libecm.la -rpath /data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib libecm_la-ecm.lo libecm_la-ecm2.lo libecm_la-pm1.lo libecm_la-pp1.lo libecm_la-getprime_r.lo libecm_la-listz.lo libecm_la-lucas.lo libecm_la-stage2.lo libecm_la-mpmod.lo libecm_la-mul_lo.lo libecm_la-polyeval.lo libecm_la-median.lo libecm_la-schoen_strass.lo libecm_la-ks-multiply.lo libecm_la-rho.lo libecm_la-bestd.lo libecm_la-auxlib.lo libecm_la-random.lo libecm_la-factor.lo libecm_la-sp.lo libecm_la-spv.lo libecm_la-spm.lo libecm_la-mpzspm.lo libecm_la-mpzspv.lo libecm_la-ntt_gfp.lo libecm_la-ecm_ntt.lo libecm_la-pm1fs2.lo libecm_la-sets_long.lo libecm_la-auxarith.lo libecm_la-batch.lo libecm_la-parametrizations.lo libecm_la-cudawrapper.lo aprtcle/libecm_la-mpz_aprcl.lo   ./x86_64/libmulredc.la  -lrt -lm -lm -lm -lm -lm
libtool: link: gcc-6.1 -shared  -fPIC -DPIC  .libs/libecm_la-ecm.o .libs/libecm_la-ecm2.o .libs/libecm_la-pm1.o .libs/libecm_la-pp1.o .libs/libecm_la-getprime_r.o .libs/libecm_la-listz.o .libs/libecm_la-lucas.o .libs/libecm_la-stage2.o .libs/libecm_la-mpmod.o .libs/libecm_la-mul_lo.o .libs/libecm_la-polyeval.o .libs/libecm_la-median.o .libs/libecm_la-schoen_strass.o .libs/libecm_la-ks-multiply.o .libs/libecm_la-rho.o .libs/libecm_la-bestd.o .libs/libecm_la-auxlib.o .libs/libecm_la-random.o .libs/libecm_la-factor.o .libs/libecm_la-sp.o .libs/libecm_la-spv.o .libs/libecm_la-spm.o .libs/libecm_la-mpzspm.o .libs/libecm_la-mpzspv.o .libs/libecm_la-ntt_gfp.o .libs/libecm_la-ecm_ntt.o .libs/libecm_la-pm1fs2.o .libs/libecm_la-sets_long.o .libs/libecm_la-auxarith.o .libs/libecm_la-batch.o .libs/libecm_la-parametrizations.o .libs/libecm_la-cudawrapper.o aprtcle/.libs/libecm_la-mpz_aprcl.o  -Wl,--whole-archive ./x86_64/.libs/libmulredc.a -Wl,--no-whole-archive  -lgmp -L/data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib -lrt -lm  -g -march=native -g -O3 -g -Wl,-z -Wl,noexecstack -Wl,-rpath -Wl,/data/Sage/release/devel/sage-7.3.rc0-gcc-6.1.0/local/lib   -Wl,-soname -Wl,libecm.so.1 -o .libs/libecm.so.1.0.0
libtool: link: (cd ".libs" && rm -f "libecm.so.1" && ln -s "libecm.so.1.0.0" "libecm.so.1")
libtool: link: (cd ".libs" && rm -f "libecm.so" && ln -s "libecm.so.1.0.0" "libecm.so")
libtool: link: (cd .libs/libecm.lax/libmulredc.a && ar x "/tmp/sage-build-tmp/ecm-7.0.3/src/./x86_64/.libs/libmulredc.a")
libtool: link: ar cru .libs/libecm.a  libecm_la-ecm.o libecm_la-ecm2.o libecm_la-pm1.o libecm_la-pp1.o libecm_la-getprime_r.o libecm_la-listz.o libecm_la-lucas.o libecm_la-stage2.o libecm_la-mpmod.o libecm_la-mul_lo.o libecm_la-polyeval.o libecm_la-median.o libecm_la-schoen_strass.o libecm_la-ks-multiply.o libecm_la-rho.o libecm_la-bestd.o libecm_la-auxlib.o libecm_la-random.o libecm_la-factor.o libecm_la-sp.o libecm_la-spv.o libecm_la-spm.o libecm_la-mpzspm.o libecm_la-mpzspv.o libecm_la-ntt_gfp.o libecm_la-ecm_ntt.o libecm_la-pm1fs2.o libecm_la-sets_long.o libecm_la-auxarith.o libecm_la-batch.o libecm_la-parametrizations.o libecm_la-cudawrapper.o aprtcle/libecm_la-mpz_aprcl.o  .libs/libecm.lax/libmulredc.a/mulredc1.o .libs/libecm.lax/libmulredc.a/mulredc10.o .libs/libecm.lax/libmulredc.a/mulredc11.o .libs/libecm.lax/libmulredc.a/mulredc12.o .libs/libecm.lax/libmulredc.a/mulredc13.o .libs/libecm.lax/libmulredc.a/mulredc14.o .libs/libecm.lax/libmulredc.a/mulredc15.o .libs/libecm.lax/libmulredc.a/mulredc16.o .libs/libecm.lax/libmulredc.a/mulredc17.o .libs/libecm.lax/libmulredc.a/mulredc18.o .libs/libecm.lax/libmulredc.a/mulredc19.o .libs/libecm.lax/libmulredc.a/mulredc1_10.o .libs/libecm.lax/libmulredc.a/mulredc1_11.o .libs/libecm.lax/libmulredc.a/mulredc1_12.o .libs/libecm.lax/libmulredc.a/mulredc1_13.o .libs/libecm.lax/libmulredc.a/mulredc1_14.o .libs/libecm.lax/libmulredc.a/mulredc1_15.o .libs/libecm.lax/libmulredc.a/mulredc1_16.o .libs/libecm.lax/libmulredc.a/mulredc1_17.o .libs/libecm.lax/libmulredc.a/mulredc1_18.o .libs/libecm.lax/libmulredc.a/mulredc1_19.o .libs/libecm.lax/libmulredc.a/mulredc1_2.o .libs/libecm.lax/libmulredc.a/mulredc1_20.o .libs/libecm.lax/libmulredc.a/mulredc1_3.o .libs/libecm.lax/libmulredc.a/mulredc1_4.o .libs/libecm.lax/libmulredc.a/mulredc1_5.o .libs/libecm.lax/libmulredc.a/mulredc1_6.o .libs/libecm.lax/libmulredc.a/mulredc1_7.o .libs/libecm.lax/libmulredc.a/mulredc1_8.o .libs/libecm.lax/libmulredc.a/mulredc1_9.o .libs/libecm.lax/libmulredc.a/mulredc2.o .libs/libecm.lax/libmulredc.a/mulredc20.o .libs/libecm.lax/libmulredc.a/mulredc3.o .libs/libecm.lax/libmulredc.a/mulredc4.o .libs/libecm.lax/libmulredc.a/mulredc5.o .libs/libecm.lax/libmulredc.a/mulredc6.o .libs/libecm.lax/libmulredc.a/mulredc7.o .libs/libecm.lax/libmulredc.a/mulredc8.o .libs/libecm.lax/libmulredc.a/mulredc9.o
libtool: link: ranlib .libs/libecm.a
libtool: link: rm -fr .libs/libecm.lax
libtool: link: ( cd ".libs" && rm -f "libecm.la" && ln -s "../libecm.la" "libecm.la" )

(env ECM_CONFIGURE="--enable-shared" ./sage -f ecm)

comment:99 in reply to: ↑ 98 Changed 4 years ago by leif

Replying to leif:

Replying to fbissey:

In Makefile.am you want to change line 52 which already has a definition for libecm_la_LDFLAGS.

Oooops, forgot a plus, but for whatever reason, it worked for me (with version info etc.).

No, after taking a closer look, I did not forget the plus. So nothing mysterious going on.

comment:100 Changed 4 years ago by fbissey

But _I_ missed it :) real time for bed for me.

comment:101 Changed 4 years ago by leif

Dreaming of libtool I guess... ;-)

comment:102 Changed 4 years ago by leif

Oh-oh, it seems the last patch brings back ar cru libecm.a libgmp.a ... when building only a static library...

comment:103 Changed 4 years ago by fbissey

I think we are majorly bike shedding.

Things to do: 1) -z noexecstack is a gnu ld option. Doesn't work on OS X. Change this and get it upstream.

2) if you provide enough info to configure to get the path to libgmp.a (not just detecting that you can link to gmp) and you build a static libecm then the build system will try to include libgmp.a in libecm.a.

  • this fails on OS X
  • if you build both a static and dynamic library interesting things will probably happen to the dynamic library, especially if libgmp.a has not been compiled with pic flags.

8 this is a recipe to squeeze as much performance as possible by getting an all static linking - it is possible to get the ecm binary compiled all static in some more portable ways (I think).

Number (2) is occurring in sage because we pass the install path to gmp/mpir when it is not needed to get gmp detected. This is my comment:76.

(1) has to be talked about with upstream (2) can easily build by adopting comment:76 in spkg-install.

comment:104 follow-up: Changed 4 years ago by leif

Yep.

For 1) we can just patch out -Wl,znoexecstack here and add it (or -Wa,--noexecstack) back in spkg-install when appropriate (i.e., supported; presumably just Linux or probably [recent] GAS).

And submit something upstream later.

To 2 ), dropping --with-gmp should IMHO only be a temporary work-around.

comment:105 in reply to: ↑ 104 ; follow-up: Changed 4 years ago by leif

Replying to leif:

To 2), dropping --with-gmp should IMHO only be a temporary work-around.

Is this sufficient or do we (still) have to patch out (the apparently plain wrong, with a static library at least)

libecm_la_LIBADD += $(GMPLIB)

?

I'll be back in a couple of hours...

comment:106 in reply to: ↑ 105 Changed 4 years ago by fbissey

Replying to leif:

Replying to leif:

To 2), dropping --with-gmp should IMHO only be a temporary work-around.

Is this sufficient or do we (still) have to patch out (the apparently plain wrong, with a static library at least)

libecm_la_LIBADD += $(GMPLIB)

?

I'll be back in a couple of hours...

It is sufficient. I don't think Makefile.am is wrong. It is just feed something toxic in a corner case which upstream goes to great length to create.

Generally speaking OS X doesn't like static libraries much (still doable but some things are made difficult or more limited). So it probably should be avoided there.

comment:107 Changed 4 years ago by leif

[Post to wrong ticket deleted.]

Last edited 4 years ago by leif (previous) (diff)

comment:108 Changed 4 years ago by leif

Ouch, wrong ticket... :-)

comment:109 follow-up: Changed 4 years ago by leif

Just for the record, #21223 adds a Cygwin patch to the 6.4.4 package.

comment:110 in reply to: ↑ 109 Changed 4 years ago by leif

  • Status changed from needs_review to needs_work

Replying to leif:

Just for the record, #21223 adds a Cygwin patch to the 6.4.4 package.

... and we need to (trivially) backport that to 7.0.3 at least.

(Patches configure[.{in,ac}], single-line change each.)

comment:111 follow-up: Changed 3 years ago by zimmerma

I'm back from vacation, and I see about 100 messages in this ticket, woowww!!!! The 7.0.2 and 7.0.3 releases now each have a tag. Please tell me if you have still issues with 7.0.3, and some patches to be applied upstream.

Paul

comment:112 in reply to: ↑ 111 Changed 3 years ago by fbissey

Replying to zimmerma:

I'm back from vacation, and I see about 100 messages in this ticket, woowww!!!! The 7.0.2 and 7.0.3 releases now each have a tag. Please tell me if you have still issues with 7.0.3, and some patches to be applied upstream.

Paul

I guess there are two main things we'll have to deal with:

  • -Wl,znoexecstack only works with gnu ld. It causes OS X ld to choke.
  • building against libgmp.a is also not working on OS X. That can be filtered out in configure.ac with an extra test in the logic to select pathto/libgmp.a

That's on top of my head.

comment:113 in reply to: ↑ 15 Changed 3 years ago by zimmerma

Apparently also building out-of-tree is broken (more precisely, running make longcheck in an out-of-tree build, which definitely fails).

should be fixed in revision 2974.

Paul

comment:114 Changed 3 years ago by zimmerma

should be fixed in revision 2974.

sorry, it should read 2975.

Paul

comment:115 follow-up: Changed 3 years ago by zimmerma

for the -Wl,znoexecstack issue, I've added a configure test in revision 2978. Please can you check it works on OS X now?

For the libgmp.a issue, please can you provide a patch?

Paul

comment:116 Changed 3 years ago by jpflori

Posting back the issue I reported on #21223:

  • on cygwin64, the assebmly for redc uses the GOT which is not supported on Windows when WANT_ASSERT is set and it is by default (passing --disable-assert circumvents the problem).

I'll now try different combinations of --enable-asm-redc and --enable-sse2 on Cygwin32 and Cygwin64 and report.

comment:117 follow-up: Changed 3 years ago by jpflori

(Do we need a so complicated spkg-install???)

comment:118 Changed 3 years ago by jpflori

On my side, make check passes on a x86_64 machine with Cygwin32 passing --enable-sse2 --enable-asm-redc.

Same under Cygwin64 (modulo rebasing issues and passing --disable-assert).

comment:119 in reply to: ↑ 117 Changed 3 years ago by fbissey

Replying to jpflori:

(Do we need a so complicated spkg-install???)

I personally don't think so... Too much hand holding on flags in my opinion.

comment:120 in reply to: ↑ 115 Changed 3 years ago by fbissey

Replying to zimmerma:

for the -Wl,znoexecstack issue, I've added a configure test in revision 2978. Please can you check it works on OS X now?

I'll give it go when I have time (hopefully in the next few hours).

For the libgmp.a issue, please can you provide a patch?

Yes, I'll definitely work on that.

Francois

comment:121 Changed 3 years ago by fbissey

On revision 2978 on OS X inside sage

configure: Memory debugging disabled

Now building GMP-ECM...
make  all-recursive
Making all in x86_64
m4 -I../ -DOPERATION_mulredc1 `test -f mulredc1.asm || echo './'`mulredc1.asm >mulredc1.s
/bin/sh ../libtool    --mode=compile gcc  -march=native -g -O3  -fPIC -c -o mulredc1.lo mulredc1.s
libtool: compile:  gcc -march=native -g -O3 -fPIC -c mulredc1.s -o mulredc1.o
mulredc1.s:59:2: error: unsupported symbol modifier in branch relocation
        call    abort@plt
        ^
make[2]: *** [mulredc1.lo] Error 1
rm mulredc1.s
make[1]: *** [all-recursive] Error 1
make: *** [all] Error 2
Error building GMP-ECM.

Ouch...

comment:122 Changed 3 years ago by jpflori

Pass --disable-assert :p

comment:123 follow-up: Changed 3 years ago by fbissey

Thanks, that worked. Will we have to pass it for anything other than linux? Or should it be magically set?

In any case the main test was whether --noexecstack was properly disabled on OS X and it is.

comment:124 in reply to: ↑ 123 Changed 3 years ago by jpflori

Replying to fbissey:

Thanks, that worked. Will we have to pass it for anything other than linux? Or should it be magically set?

I would say it's a bug. The assembly should be modified because:

  • GOT is not supported on Windows (where everything is alway PIC),
  • the call syntax is broken on OS X.

Using --disable-assert is just a workaround as it excludes the harming code. Note that we could still live without asserts on Cygwin and OS X.

In any case the main test was whether --noexecstack was properly disabled on OS X and it is.

Good!

comment:125 follow-ups: Changed 3 years ago by zimmerma

about the issue from comment 121 (GOT not supported on Windows), I can't help on that. Please can someone provide a patch?

Since the noexecstack issue is now solved in the development version, it only remains to solve the libgmp.a issue, for which Francois is working on a patch.

Paul

comment:126 in reply to: ↑ 125 Changed 3 years ago by jpflori

Replying to zimmerma:

about the issue from comment 121 (GOT not supported on Windows), I can't help on that. Please can someone provide a patch?

Note that the assembly line from 121 gives issues on OS X not Windows:

call abort@plt

The problematic line on Windows from #21223 is the previous one which uses:

$_GLOBAL_OFFSET_TABLE

The workaround I suggested is to use --disable-assert as both these lines are surrounded by a (m4-equivalent) of #ifdef HAVE_ASSERT.

A proper fix would be to:

  • find equivalent assembly on OS X and Windows to keep asserts support,
  • or always disable this code path on OS X and Windows.

comment:127 Changed 3 years ago by jpflori

This comes from: https://gforge.inria.fr/scm/viewvc.php/ecm/trunk/x86_64/mulredc1.asm?r1=1460&r2=1526 and yes it is not portable!

In fact this only appeared here because in configure.ac (https://gforge.inria.fr/scm/viewvc.php/ecm/trunk/configure.ac?view=markup#l51):

dnl Assertions are enabled by default for beta/rc releases.

comment:128 Changed 3 years ago by zimmerma

thank you Jean-Pierre. If we go back to revision 1460, would it solve the OS X and Windows issues?

comment:129 Changed 3 years ago by jpflori

Not completely sure if you mean go back to r1460 for OS X and Windows only or for all Oses.

Whatsoever, with no testing and realknowledge of what I'm talking about, I'd say that indeed using:

  • using the r1460 code on Windows should do the trick: there the code is always PIC and hopefully no dark magic is needed to call abort;
  • using the r1460 code on OS X I have no idea but it cannot be worse than now: binaries are not ELF, so no PLT, not sure if the simple call would always work in both shared and static libraries.

As far as Linux is concerned :

  • either always use the r1526 code which according to the commit message always works,
  • or use it only in PIC code and use the r1460 in non-PIC code.

comment:130 Changed 3 years ago by zimmerma

I was meaning go back to r1460 in all cases. But I now propose to simply remove this WANT_ASSERT stuff in the assembly code. Anyway if something goes wrong, it will be detected by the assertions in the C code.

Paul

comment:131 Changed 3 years ago by jpflori

Sure that's the more portable and easiest to maintain solution.

(And after a few more seconds searching about OS X, it might be the case that the toolchain there does the plt or equivalent black magic by itself.)

comment:132 Changed 3 years ago by zimmerma

the WANT_ASSERT stuff has been removed in upstream revision 2981. This should solve the OS X and Windows issues.

Paul

comment:133 in reply to: ↑ 125 Changed 3 years ago by fbissey

Replying to zimmerma:

about the issue from comment 121 (GOT not supported on Windows), I can't help on that. Please can someone provide a patch?

Since the noexecstack issue is now solved in the development version, it only remains to solve the libgmp.a issue, for which Francois is working on a patch.

Paul

Sent your way in a private email.

Francois

comment:134 Changed 3 years ago by zimmerma

the patch for the libgmp.a issue on OS X is integrated in upstream revision 2982. Are all known issues solved now?

Paul

PS: I'll probably make a 7.0.4 patch release since a bug was reported in mpres_pow().

comment:135 Changed 3 years ago by zimmerma

a new release candidate is available at

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

Please test and report success or failure.

Paul

comment:136 Changed 3 years ago by fbissey

I still need --disable-assert on OS X otherwise I get

/bin/sh ./libtool  --tag=CC   --mode=link gcc  -g -march=native -g -O3  -fPIC  -L/Users/fbissey/build/sage-7.2.beta5/local/lib -Wl,-rpath,/Users/fbissey/build/sage-7.2.beta5/local/lib  -L/Users/fbissey/build/sage-7.2.beta5/local/lib -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o libecm.la -lgmp -lm -lm -lm -lm -lm  
libtool: link: gcc -g -march=native -g -O3 -fPIC -Wl,-rpath -Wl,/Users/fbissey/build/sage-7.2.beta5/local/lib -o ecm ecm-auxi.o ecm-b1_ainc.o ecm-candi.o ecm-eval.o ecm-main.o ecm-resume.o ecm-getprime_r.o aprtcle/ecm-mpz_aprcl.o ecm-memusage.o  -L/Users/fbissey/build/sage-7.2.beta5/local/lib ./.libs/libecm.a -lgmp -lm
Undefined symbols for architecture x86_64:
  "_fft_adjust_limbs", referenced from:
      ___ecm_mpres_mul_z_to_z in libecm.a(libecm_la-mpmod.o)
      ___ecm_mpres_mul in libecm.a(libecm_la-mpmod.o)
ld: symbol(s) not found for architecture x86_64
collect2: error: ld returned 1 exit status
make[2]: *** [ecm] Error 1

ecm test suite is currently running but I am not expecting any failures.

comment:137 Changed 3 years ago by zimmerma

Francois, I guess you are compiling GMP-ECM with MPIR, not with GMP. Since there is currently no public FFT interface for GMP, and the internal functions differ between GMP and MPIR, I decided to only support the GMP side. Thus unless someone volunteers to maintain GMP-ECM up to date with changes in MPIR (apparently fft_adjust_limbs now disappeared), MPIR will not be supported any more.

Paul

comment:138 Changed 3 years ago by fbissey

Indeed I was compiling from inside a sagemath install with mpir. Which of course is the default in this case. Your comments make me wonder about what is happening on the linux side. Anyone???

comment:139 Changed 3 years ago by jpflori

As long as this only fails with --enable-assert we could live with that failure. From a quick discussion with Bill Hart, I don't see anything on the MPIR side to converge back to the GMP interface in the near future.

I'll test on Linux when I find some time this week.

comment:140 Changed 3 years ago by jpflori

I get the same failure on Linux with --enable-assert:

tune-mpmod.o: In function `__ecm_mpres_mul_z_to_z':
/home/jpflori/trunk/mpmod.c:1594: undefined reference to `fft_adjust_limbs'
tune-mpmod.o: In function `__ecm_mpres_mul':
/home/jpflori/trunk/mpmod.c:1419: undefined reference to `fft_adjust_limbs'
collect2: error: ld returned 1 exit status

No issue with --disable-assert.

comment:141 Changed 3 years ago by jpflori

Is there really any more work needed here? Wait for a new upstream dot release ? Add --disable-assert to our configuration?

comment:142 Changed 3 years ago by fbissey

I guess we should check if 7.0.4 is out and use --disable-assert.

comment:143 Changed 3 years ago by zimmerma

7.0.4 is now out.

Paul

comment:144 Changed 3 years ago by fbissey

OK, I'll try to put the branch in shape in the next 48hours.

comment:145 Changed 3 years ago by fbissey

  • Description modified (diff)

comment:146 Changed 3 years ago by jpflori

Or 48days :)

comment:147 Changed 3 years ago by fbissey

I have a branch sitting there and for some reasons I don't remember I didn't push. I'll have to examine what it is. It is not finished in any case as changes in ecm interfaces are not yet taken into account in sage. It is just the package right now.

comment:149 Changed 3 years ago by fbissey

I should compare it to the debian one. I was hoping that a debian or arch dev would finish the ticket so they can be properly credited but obviously someone else will have to.

comment:150 Changed 3 years ago by isuruf

debian patch is from arch. You can give credit by setting the git author to the author of the patch, https://git.archlinux.org/svntogit/community.git/patch/trunk/ecm-7.patch?id=e6b360003ee0d12fcc7c2a7e3eaa60bdb570325f

comment:151 Changed 3 years ago by jdemeyer

  • Branch changed from u/leif/gmp-ecm-7.x to u/jdemeyer/gmp-ecm-7.x

comment:152 Changed 3 years ago by git

  • Commit changed from e115141455e83eb0213a83d4153f72d80a767e2d to eaee6e002fd1b89dc5c63401f72a3ff93678825c

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

eaee6e0Fix libecm doctests

comment:153 Changed 3 years ago by jpflori

@jeroen: more work needed?

comment:154 in reply to: ↑ description Changed 3 years ago by jdemeyer

This has not been addressed yet:

Replying to zimmerma:

  • We definitely also have to fix the pexpect interface to ecm in src/sage/interfaces/ecm.py, as the command line interface has changed in GMP-ECM 7.x.

comment:155 Changed 3 years ago by jdemeyer

I don't plan to work more on this for now.

comment:156 Changed 3 years ago by fbissey

  • Authors changed from Leif Leonhardy to Leif Leonhardy, Jeroen Demeyer, François Bissey, Antonio Rojas
  • Branch changed from u/jdemeyer/gmp-ecm-7.x to u/fbissey/gmp-ecm-7.x
  • Commit changed from eaee6e002fd1b89dc5c63401f72a3ff93678825c to dde736b3c5382de6617fadd7597e4f48131af4cb
  • Status changed from needs_work to needs_review

Adding the patch from arch linux (and credit the author properly) and let's try to finish this.


New commits:

dea25cdMerge branch 'develop' into gmp-ecm-7.x
dde736bAdd interface patch for emc 7 from the Arch linux port. The Arch commit is credited to Antonio Rojas (arojas@archlinux.org)

comment:157 Changed 3 years ago by jpflori

Thanks I was working on this but it will be faster to just review this patch.

comment:158 Changed 3 years ago by jpflori

  • Reviewers set to Jean-Pierre Flori
  • Status changed from needs_review to positive_review
  • Work issues Fix doctests and pexpect interface to ecm deleted

comment:159 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Testsuite fails:

[ecm-7.0.4.p0] ********** Factor found in step 2: 4190453151940208656715582382315221647
[ecm-7.0.4.p0] Found prime factor of 37 digits: 4190453151940208656715582382315221647
[ecm-7.0.4.p0] Prime cofactor 543423179434447039008165356160798838947285203071935410761431031147 has 66 digits
[ecm-7.0.4.p0] GMP-ECM 7.0.4 [configured with MPIR 2.7.2, --enable-asm-redc] [P+1]
[ecm-7.0.4.p0] Save file test.pp1.save already exists, will not overwrite
[ecm-7.0.4.p0] ############### ERROR ###############
[ecm-7.0.4.p0] Expected return code 0 but got 1
[ecm-7.0.4.p0] Makefile:2521: recipe for target 'longcheck' failed
[ecm-7.0.4.p0] make[3]: *** [longcheck] Error 1
[ecm-7.0.4.p0] PASS: test.pm1
[ecm-7.0.4.p0] PASS: test.pp1
[ecm-7.0.4.p0] PASS: test.ecm
[ecm-7.0.4.p0] ============================================================================
[ecm-7.0.4.p0] Testsuite summary for ecm 7.0.4
[ecm-7.0.4.p0] ============================================================================
[ecm-7.0.4.p0] # TOTAL: 3
[ecm-7.0.4.p0] # PASS:  3
[ecm-7.0.4.p0] # SKIP:  0
[ecm-7.0.4.p0] # XFAIL: 0
[ecm-7.0.4.p0] # FAIL:  0
[ecm-7.0.4.p0] # XPASS: 0
[ecm-7.0.4.p0] # ERROR: 0
[ecm-7.0.4.p0] ============================================================================
[ecm-7.0.4.p0] make[6]: Leaving directory '/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p0/src'
[ecm-7.0.4.p0] make[5]: Leaving directory '/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p0/src'
[ecm-7.0.4.p0] make[4]: Leaving directory '/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p0/src'
[ecm-7.0.4.p0] make[3]: Leaving directory '/mnt/disk/home/buildslave-sage/slave/sage_git/build/local/var/tmp/sage/build/ecm-7.0.4.p0/src'
[ecm-7.0.4.p0] Error: GMP-ECM's test suite failed.

comment:160 Changed 3 years ago by fbissey

Looks like a race condition in the test suite.

comment:161 Changed 3 years ago by fbissey

Yes, from Makefile.am

longcheck: ecm$(EXEEXT)
        $(srcdir)/test.pp1 "$(VALGRIND) ./ecm$(EXEEXT)"
        $(srcdir)/test.pp1 "$(VALGRIND) ./ecm$(EXEEXT) -no-ntt"
        $(srcdir)/test.pp1 "$(VALGRIND) ./ecm$(EXEEXT) -modmuln"
        $(srcdir)/test.pp1 "$(VALGRIND) ./ecm$(EXEEXT) -redc"
        $(srcdir)/test.pp1 "$(VALGRIND) ./ecm$(EXEEXT) -mpzmod"
        $(srcdir)/test.pm1 "$(VALGRIND) ./ecm$(EXEEXT)"
        $(srcdir)/test.pm1 "$(VALGRIND) ./ecm$(EXEEXT) -no-ntt"
        $(srcdir)/test.pm1 "$(VALGRIND) ./ecm$(EXEEXT) -modmuln"
        $(srcdir)/test.pm1 "$(VALGRIND) ./ecm$(EXEEXT) -redc"
        $(srcdir)/test.pm1 "$(VALGRIND) ./ecm$(EXEEXT) -mpzmod"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT)"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT) -no-ntt"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT) -modmuln"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT) -redc"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT) -mpzmod"
        $(srcdir)/test.ecm "$(VALGRIND) ./ecm$(EXEEXT) -treefile tree"
        $(srcdir)/testlong.pp1 "$(VALGRIND) ./ecm$(EXEEXT)"
        $(srcdir)/testlong.pm1 "$(VALGRIND) ./ecm$(EXEEXT)"
        $(srcdir)/testlong.ecm "$(VALGRIND) ./ecm$(EXEEXT)"

and on both bots the test are probably with -j8. Each instance of test.pp1 can write test.pp1.save. There could be similar problem with other tests. The only solution, I think, is to run the longcheck serially.

comment:162 Changed 3 years ago by git

  • Commit changed from dde736b3c5382de6617fadd7597e4f48131af4cb to 946c5808691c5437cb6b2148fe0a5b7a200ce140

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

946c580making the longcheck tests run serially

comment:163 Changed 3 years ago by fbissey

  • Status changed from needs_work to positive_review

Putting back to positive review to send back to the bots.

comment:164 Changed 3 years ago by vbraun

  • Branch changed from u/fbissey/gmp-ecm-7.x to 946c5808691c5437cb6b2148fe0a5b7a200ce140
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:165 Changed 3 years ago by zimmerma

  • Commit 946c5808691c5437cb6b2148fe0a5b7a200ce140 deleted

for the record, I have fixed upstream (svn revision 3022) so that we can now run make check in parallel. You could also apply the corresponding patch.

comment:166 Changed 3 years ago by fbissey

I see, you made all those little checkpointing files unique rather than re-using the same one all the time. Well this ticket is closed but we could have a follow up. And hopefully it won't languish for as much time as this one.

comment:167 Changed 3 years ago by vbraun

Followup at #23013

comment:168 Changed 3 years ago by jdemeyer

Since this hasn't been released yet, why not just unmerge this?

comment:169 Changed 3 years ago by vbraun

No particular reason...

Note: See TracTickets for help on using tickets.