Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#23225 closed defect (fixed)

fix tuning of gf2x

Reported by: dimpase Owned by:
Priority: blocker Milestone: sage-8.0
Component: packages: standard Keywords:
Cc: zimmerma, tscrim Merged in:
Authors: Paul Zimmermann, Dima Pasechnik Reviewers: Volker Braun
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: 3761c47 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

see the last comments on #18882

There are a wrong ASSERT in toom2.c that errors out on some systems. In addition, tuning routine may link against a wrong copy of libgf2x.so.

Change History (26)

comment:1 Changed 4 years ago by dimpase

  • Authors set to Paul Zimmermann, Dima Pasechnik
  • Branch set to u/dimpase/gf2xfix
  • Commit set to e6b5456ff1d6178695dab727873895a6dc779aac
  • Status changed from new to needs_review

New commits:

e6b5456fixes wrong ASSERT, as proposed by Paul on #18882

comment:2 Changed 4 years ago by git

  • Commit changed from e6b5456ff1d6178695dab727873895a6dc779aac to 3761c47a88d477378b0633fe50dcca8ccf57bb64

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

3761c47remove old files before building, not after

comment:3 Changed 4 years ago by fbissey

Yes the removal of the old libraries may be necessary in sage. But I don't think it should, because the linked test executable are not executed directly but through a libtool script. And that libtool script should set {DY}LD_LIBRARY_PATH to the the built library. I guess we could examine one of them to be sure there is no pollution from somewhere but {DY,}LD_LIBRARY_PATH takes precedence to rpath set during compilation.

comment:4 Changed 4 years ago by dimpase

  • Cc zimmerma tscrim added
  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

comment:5 Changed 4 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:6 Changed 4 years ago by vbraun

  • Branch changed from u/dimpase/gf2xfix to 3761c47a88d477378b0633fe50dcca8ccf57bb64
  • Resolution set to fixed
  • Status changed from positive_review to closed

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

  • Commit 3761c47a88d477378b0633fe50dcca8ccf57bb64 deleted

Hi,

I'm reviewing the suggested changes to gf2x in order to prepare a release.

Several issues were raised in #18882.

I don't see the point about shared libs possibly linking to an outdated version. I haven't witnessed that. If I install some libgf2x.so in the library path (say, in /usr/local), then we have the following.

On the one hand, it's true that "ldd ./src/.libs/tunetoom" sees the /usr/local library as a way to resolve the dependency:

  libgf2x.so.1 => /usr/local/lib/libgf2x.so.1

On the other hand, the above is a non-issue, because in the testing procedure, libtool takes care of ensuring that the libgf2x which gets linked is the right one. I check that with

  cat /proc/`pidof tunetoom`/maps

while tunetoom is running. It does use the one in the build tree, not the /usr/local one.

Is there something else that I've missed in the way you encounter a misbehaviour ?

comment:8 Changed 4 years ago by fbissey

@ thome, I agree with you libtool wraps the command before running it. It is not foolproof but usually work well. It is not foolproof because stray .la files can introduce system path in {DY,}LD_LIBRARY_PATH to not so hilarious effects. But I think here, dima may not have known about libtool wrapping - which means that it looks like you are linking to a library in a system path.

comment:9 in reply to: ↑ 7 ; follow-up: Changed 4 years ago by dimpase

Replying to thome:

Hi,

I'm reviewing the suggested changes to gf2x in order to prepare a release.

Several issues were raised in #18882.

I don't see the point about shared libs possibly linking to an outdated version. I haven't witnessed that. If I install some libgf2x.so in the library path (say, in /usr/local), then we have the following.

On the one hand, it's true that "ldd ./src/.libs/tunetoom" sees the /usr/local library as a way to resolve the dependency:

  libgf2x.so.1 => /usr/local/lib/libgf2x.so.1

On the other hand, the above is a non-issue, because in the testing procedure, libtool takes care of ensuring that the libgf2x which gets linked is the right one. I check that with

  cat /proc/`pidof tunetoom`/maps

while tunetoom is running. It does use the one in the build tree, not the /usr/local one.

Is there something else that I've missed in the way you encounter a misbehaviour ?

Maybe this only works when you have something in a system path; in my case I did see this wrong linking to SAGE_LOCAL/lib/libgf2x.so.1 rather than to the libgf2x.so.1 currently being built.

It may also be that FreeBSD's linker behaves differently and does the wrong linking regardless...

Anyway, I would be happy to test a newer version of gf2x on FreeBSD, if you don't have access to such a system. I have just updated the relevant ticket #22679.

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

OK, I am not actually sure that LD_LIBRARY_PATH, which is used by the libtool wrapper, works on freeBSD.

comment:11 in reply to: ↑ 10 ; follow-up: Changed 4 years ago by dimpase

Replying to fbissey:

OK, I am not actually sure that LD_LIBRARY_PATH, which is used by the libtool wrapper, works on freeBSD.

IMHO LD_LIBRARY_PATH is certainly a problem on OSX, thus a better way needs to be found...

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

Replying to dimpase:

Replying to fbissey:

OK, I am not actually sure that LD_LIBRARY_PATH, which is used by the libtool wrapper, works on freeBSD.

IMHO LD_LIBRARY_PATH is certainly a problem on OSX, thus a better way needs to be found...

libtool is smart enough to use DYLD_LIBRARY_PATH on OS X, so that's OK. But freeBSD is an unknown, I don't know if it support any mechanism of that kind.

comment:13 Changed 4 years ago by vbraun

Depending on the linker (--enable-new-dtags or not), either RPATH or LD_LIBRARY_PATH has precedence.

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

Replying to dimpase:

Maybe this only works when you have something in a system path; in my case I did see this wrong linking to SAGE_LOCAL/lib/libgf2x.so.1 rather than to the libgf2x.so.1 currently being built.

It's hard to work with maybe's...

I can test the following.

  1. install some gf2x in a system path
  2. run gf2x tuning from a fresh tarball
  3. make sure the tuning does not use the lib from step 1.

I can also replace step 1. by "install some gf2x in a custom path, and enable its use via LD_LIBRARY_PATH".

I can do this test on various machines. I understand you've seen issues on freebsd, I can do a test on freebsd 11.0.

I can also test on macosx 11.0

However if the procedure that led you to a misbehaviour is not within the scope of what I will be testing, chances are that I'm not going to detect anything wrong.

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

Replying to fbissey:

Replying to dimpase:

Replying to fbissey:

OK, I am not actually sure that LD_LIBRARY_PATH, which is used by the libtool wrapper, works on freeBSD.

IMHO LD_LIBRARY_PATH is certainly a problem on OSX, thus a better way needs to be found...

Would you mind being more precise about what you mean by "a problem" ?

AFAIK OSX uses DYLD_LIBRARY_PATH. And so what ?

It should not be within the scope of a package I write to deal with the idiosyncrasies of all unices on earth. Libtool is for that, and precisely for that.

libtool is smart enough to use DYLD_LIBRARY_PATH on OS X, so that's OK. But freeBSD is an unknown, I don't know if it support any mechanism of that kind.

Yup.

It's certainly not as if freeBSD was a new kid on the block, you know... That it would be "unsupported" by libtool in some way would surprise me very much.

E.

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

Replying to thome:

Replying to dimpase:

Maybe this only works when you have something in a system path; in my case I did see this wrong linking to SAGE_LOCAL/lib/libgf2x.so.1 rather than to the libgf2x.so.1 currently being built.

It's hard to work with maybe's...

I can test the following.

  1. install some gf2x in a system path
  2. run gf2x tuning from a fresh tarball
  3. make sure the tuning does not use the lib from step 1.

I can also replace step 1. by "install some gf2x in a custom path, and enable its use via LD_LIBRARY_PATH".

I presume that by 3. you mean "break the installed gf2x, so that tuning---if it uses it---will fail".

I can do this test on various machines. I understand you've seen issues on freebsd, I can do a test on freebsd 11.0.

Yes, please test on FreeBSD 11.0 with CC=clang and CXX=clang++ (I think I saw this with clang version 4.0.0; probably the current (3.8.0) would reproduce this too).

And also note that I am talking about gf2x-1.1. I did not try the development version yet.

I can also test on macosx 11.0

However if the procedure that led you to a misbehaviour is not within the scope of what I will be testing, chances are that I'm not going to detect anything wrong.

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

Replying to thome:

Replying to fbissey:

Replying to dimpase:

Replying to fbissey:

OK, I am not actually sure that LD_LIBRARY_PATH, which is used by the libtool wrapper, works on freeBSD.

IMHO LD_LIBRARY_PATH is certainly a problem on OSX, thus a better way needs to be found...

Would you mind being more precise about what you mean by "a problem" ?

I merely meant to say that direct manipulation of LD_LIBRARY_PATH on OSX is not really possible, as by default OSX has some security thing turned on that basically blocks it.

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

Replying to dimpase:

Replying to thome:

  1. install some gf2x in a system path
  2. run gf2x tuning from a fresh tarball
  3. make sure the tuning does not use the lib from step 1.

[...]

I presume that by 3. you mean "break the installed gf2x, so that tuning---if it uses it---will fail".

I rather had in mind a live check that tuning binaries run with the proper lib. This being said you're right. While it's trivial to do on linux with /proc/pid xxx/maps, it's maybe less so on other systems. So I'll probably be better off sticking an abort() in the installed gf2x and see fireworks (if any).

Yes, please test on FreeBSD 11.0 with CC=clang and CXX=clang++ (I think I saw this with clang version 4.0.0; probably the current (3.8.0) would reproduce this too).

ok thanks. Good to know.

Struggling with our cloudstack server at the moment, but I still hope I'll be able to do that shortly.

And also note that I am talking about gf2x-1.1. I did not try the development version yet.

I understood that. I really mean to make a release tarball, so I'm gonna check with the development version. I'll give a short go to the 1.1 version to see if I can confirm a defect.

E.

comment:19 in reply to: ↑ 17 Changed 4 years ago by thome

Replying to dimpase:

I merely meant to say that direct manipulation of LD_LIBRARY_PATH on OSX is not really possible, as by default OSX has some security thing turned on that basically blocks it.

Ah ok, that's more clear now.

Sigh.

I knew about OSX preventing gdb for non-root users. Now LD_LIBRARY_PATH. What next ?

But anyway, as I was saying I'm convinced that I should not have to bother with library path tricks, given that nothing which is done by gf2x is super-weird to that respect.

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

Replying to thome:

Replying to dimpase:

Replying to thome:

  1. install some gf2x in a system path
  2. run gf2x tuning from a fresh tarball
  3. make sure the tuning does not use the lib from step 1.

[...]

I presume that by 3. you mean "break the installed gf2x, so that tuning---if it uses it---will fail".

I rather had in mind a live check that tuning binaries run with the proper lib. This being said you're right. While it's trivial to do on linux with /proc/pid xxx/maps, it's maybe less so on other systems. So I'll probably be better off sticking an abort() in the installed gf2x and see fireworks (if any).

Basically, I was changing the tuning code and not seeing any difference, until I checked the tuning routine with ldd and saw it linking to the wrong copy of the dynamic library.

Sorry for noise---no need to run any "live" tests, just build and check with ldd that lt-tunetoom is linked against the right library.

comment:21 in reply to: ↑ 20 Changed 4 years ago by thome

Replying to dimpase:

Basically, I was changing the tuning code and not seeing any difference, until I checked the tuning routine with ldd and saw it linking to the wrong copy of the dynamic library.

Sorry for noise---no need to run any "live" tests, just build and check with ldd that lt-tunetoom is linked against the right library.

You're tinkering with libtool's internals, there. Theoretically, libtool's wrapper could wrap the final executable in such a way that the library paths you see with ldd on .libs/blah are irrelevant.

This being said, I've been able to reproduce your freebsd problem. I'll look into it a bit more.

comment:22 Changed 4 years ago by thome

So. This "tuning hits the wrong library" issue is a libtool bug IMHO.

I've committed a workaround to gf2x, but the cause of the misbehaviour is completely unrelated to gf2x itself. If you follow that same procedure of leaving an installed lib, and try to run the tuning of e.g., gmp, then you'll be testing the old library just the same.

https://scm.gforge.inria.fr/anonscm/gitweb?p=gf2x/gf2x.git;a=commit;h=799afa342c1cf744cacd6987f28b9baef75faacf

libtool bug reported there.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27510

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

comment:23 Changed 4 years ago by fbissey

Thanks for getting to the bottom of it thome!

comment:24 Changed 4 years ago by dimpase

Impressive find!

comment:25 Changed 4 years ago by thome

Would you mind testing whether the latest release candidate looks good to you?

The page with all released tarballs is there: https://gforge.inria.fr/frs/?group_id=1874

rc1 did not live long, as Paul noticed a corner case.

E.

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

comment:26 Changed 4 years ago by dimpase

I've created #23357 to deal with the upgrade to gf2x version 1.2.

Note: See TracTickets for help on using tickets.