secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

build: Link `bench` binary statically

Open hebasto opened this issue 2 years ago • 5 comments

While testing bitcoin-core/secp256k1#1022 I've noticed a linker error when cross compiling with --host=x86_64-w64-mingw32 in particular circumstances.

I cannot recall my building environment then, but currently, on Ubuntu 22.04 with gcc 11.2, this error raises unconditionally:

$ ./autogen.sh && ./configure --host=x86_64-w64-mingw32 && make clean
$ make
make  all-am
make[1]: Entering directory '/home/hebasto/git/secp256k1'
  CC       src/bench.o
  CC       src/libsecp256k1_la-secp256k1.lo
  CC       src/libsecp256k1_precomputed_la-precomputed_ecmult.lo
  CC       src/libsecp256k1_precomputed_la-precomputed_ecmult_gen.lo
  CCLD     libsecp256k1_precomputed.la
  CCLD     libsecp256k1.la
  CCLD     bench.exe
libtool:   error: Could not determine the host path corresponding to
libtool:   error:   '/home/hebasto/git/secp256k1/.libs'
libtool:   error: Continuing, but uninstalled executables may not work.
libtool:   error: Could not determine the host path corresponding to
libtool:   error:   '/home/hebasto/git/secp256k1/.libs:/usr/local/lib:/usr/local/bin'
libtool:   error: Continuing, but uninstalled executables may not work.
  CC       src/bench_internal-bench_internal.o
  CCLD     bench_internal.exe
  CC       src/bench_ecmult-bench_ecmult.o
  CCLD     bench_ecmult.exe
  CC       src/tests-tests.o
  CCLD     tests.exe
  CC       src/exhaustive_tests-tests_exhaustive.o
  CCLD     exhaustive_tests.exe
make[1]: Leaving directory '/home/hebasto/git/secp256k1'

This PR prevents linker errors when cross compiling with --host=x86_64-w64-mingw32.

Please note that the same linker flag is used for all of the tests and examples. Moreover, it was used in the initial Autotools commit:https://github.com/bitcoin-core/secp256k1/blob/78cd96b15153e209cf4829a511f9efdfdcf7e4d0/Makefile.am#L45

hebasto avatar Jul 05 '22 07:07 hebasto

For what it's worth, these messages are entirely harmless.

For shared libraries, libtool has some magic which creates wrapper executables (scripts or binaries) that let you run your programs without installing the shared library or fiddling around with environment settings such as LD_LIBRARY_PATH. In this case, this means libtool creates a ./bench.exe, which is no more than a wrapper binary that sets up an environment with the correct settings and then runs the proper ./libs/bench.exe, which in turn then knows where to find the shared library in ./libs/libsecp256k1-0.dll. So you could run ./bench.exe if you happen to have wine installed.

Now the crux is that even creating the wrappers needs a filename conversion between Unix and Windows, and this requires the winepath program from wine. Because you don't have wine installed, this conversion fails. But this is not a problem because you anyway couldn't run ./bench.exe because you don't have wine installed...

The problem with -static is that it avoids the entire process of creating wrapper executables. Yes, this makes the problem disappear. But if you install wine and run ./configure --host=x86_64-w64-mingw32 --disable-static to disable static libraries entirely. Then ./bench.exe won't work, it just can't find the dll. (You may wonder why the build worked at all... This is just libtool being too clever and stripping -static when you pass --disable-static...) Same happens with the examples.

So I believe we should do the opposite: Drop -static for all binaries that should be linked against the library (as a user would do). This naturally includes all the examples but also the normal benchmark (because the bench binary benchmarks the public API only), but not the tests. In other words, there was a reason why -static was missing for the benchmarks.

@elichai Is there a specific reason why you set -static for the examples? Or was this just "copy/paste and it worked"?

real-or-random avatar Jul 05 '22 10:07 real-or-random

For what it's worth, these messages are entirely harmless.

Yes, they are. In terms of the resulted artifacts. From the user's point of view, the "error" word in the log means something wrong. It should be avoided or documented clearly.

So I believe we should do the opposite: Drop -static for all binaries that should be linked against the library (as a user would do). This naturally includes all the examples but also the normal benchmark (because the bench binary benchmarks the public API only), but not the tests. In other words, there was a reason why -static was missing for the benchmarks.

Maybe make behavior dependent on the fact whether Wine has been installed?

hebasto avatar Jul 05 '22 11:07 hebasto

Maybe make behavior dependent on the fact whether Wine has been installed?

Yeah, I'd be somewhat reluctant to maintain a workaround for this if it's not actually only a problem in the error message. I agree an "error" sounds scary but even the message then says "Continuing, but uninstalled executables may not work.". What do you think of reporting that to libtool and suggesting they change it to a warning? The manual even says it's a warning:

Failure to convert paths (see File Name Conversion Failure) will cause a warning to be issued,

libtool was dead for a long time but now they have a new maintainer and they recently tagged a release after 7 years.

real-or-random avatar Jul 05 '22 12:07 real-or-random

@elichai Is there a specific reason why you set -static for the examples? Or was this just "copy/paste and it worked"?

From what I remember it was that I saw that everything was using -static so I also added it, I also seem to remember that removing it complicates running Valgrind a bit because it runs it on the wrapper script, but I'm not sure about that.

elichai avatar Jul 06 '22 18:07 elichai

But if you install wine and run ./configure --host=x86_64-w64-mingw32 --disable-static to disable static libraries entirely. Then ./bench.exe won't work...

IIUC, we do not use such a configuration in CI. Why we should bother about it at all?

https://gnusha.org/secp256k1/2022-07-12.log

hebasto avatar Jul 12 '22 13:07 hebasto