macOS: Prevent Xcode 9.3 build warnings

[AkioNak]

This PR intends to solve #12867

1. clang (Apple LLVM version 9.1.0 (clang-902.0.39.1)) warns unused argument '-pie' during compilation. This parameter should pass to the linker, so use '-Wl,-pie' style in configure.ac.
2. The other hand, when linking libbitcoinconsensus.la with passing '-Wl,-pie', ld warns that '-pie' being ignored because not link a main executable. So remove it from $libbitcoinconsensus_la_LDFLAGS in src/Makefile.am.

• In order to apply this change to Makefile, you need to run autogen.sh and ./configure.
• I think no need to add if-clause by target-os because also g++ can recognize/handle '-Wl,-pie'.
• I confirmed that the build was successful on macos10.13.4/XCode9.3 and ubuntu 17.10/g++7.2.0 and that warning was not issued.

[kallewoof]

utACK.

It looks like -pie is on by default in the linker on GCC 6 and onward (and same for clang), so this was probably passed to the compiler instead of the linker... Either way, this PR fixes it, for systems where -pie is not enabled by default.

[l2a5b1]

Nice! Tested ACK.
MacOS 10.13.4 (17E199) and Xcode 9.3 (9E145)

[laanwj]

Is this also correct for non-macOS? I've seen -pie a lot in build systems but never -Wl,-pie.
@theuni?

[theuni]

(see edit at bottom)

The issue here is that macOS is always pie, so there's no need to specify it. The compiler front-end warns about it, but I guess the linker doesn't care.

@kallewoof default-pie is a configure option for recent compilers (at least for gcc, and I assume clang as well). It's up to distros/packagers whether they want to enable it or not. Modern distros have begun enabling it.

So the change here doesn't really fix the problem, it just bypasses the compiler and goes straight to the linker, which may also start complaining in some future version.

What we should be doing here is checking for warnings in the test instead. Windows is also default-pie and was special-cased because it also warned, but we can also eliminate that case if warnings are caught:

   AX_CHECK_LINK_FLAG([[-Wl,-z,relro]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-z,relro"])
   AX_CHECK_LINK_FLAG([[-Wl,-z,now]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-z,now"])

-  if test x$TARGET_OS != xwindows; then
-    AX_CHECK_COMPILE_FLAG([-fPIE],[PIE_FLAGS="-fPIE"])
-    AX_CHECK_LINK_FLAG([[-pie]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"])
-  fi
+  AX_CHECK_COMPILE_FLAG([-fPIE],[PIE_FLAGS="-fPIE"], [[$CXXFLAG_WERROR]]))
+  AX_CHECK_LINK_FLAG([[-pie]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"], [[$CXXFLAG_WERROR]])

   case $host in
     *mingw*)

As for libbitcoinconsensus, building it as pie is the wrong thing to be doing anyway. I'm actually not sure how that's working now. I believe the proper fix is simply:

diff --git a/src/Makefile.am b/src/Makefile.am
index 993bf20387..b5f8b2c197 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -457,7 +457,7 @@ endif
 libbitcoinconsensus_la_LDFLAGS = $(AM_LDFLAGS) -no-undefined $(RELDFLAGS)
 libbitcoinconsensus_la_LIBADD = $(LIBSECP256K1)
 libbitcoinconsensus_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/obj -I$(srcdir)/secp256k1/include -DBUILD_BITCOIN_INTERNAL
-libbitcoinconsensus_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
+libbitcoinconsensus_la_CXXFLAGS = $(AM_CXXFLAGS) $(PIC_FLAGS)

 endif
 #

Edit: On second glance, libbitcoinconsensus will still end up with -pie on linux because AM_LDFLAGS contains HARDENED_LDFLAGS. I guess we'll need to split pie out of HARDENED_LDFLAGS :(

Edit2: Researching this for what must be the hundredth time and it's just as hazy as ever. It looks like building the lib as -pie might be fine, but it'd be nice to find a definitive answer and add a comment with an explanation. If that's the case, the second patch here can just be dropped.

[AkioNak]

@theuni Thank you for your review.

[kallewoof]

@theuni Thanks for the detailed explanation! I agree that we should check for pie warnings as in your first patch. No idea on the second patch, really.

[AkioNak]

@theuni I understood that how to use 4th parameter of AX_CHECK_{COMPILE,LINK}_FLAG().
I agree what you pointed out. thanks.

[theuni]

@kallewoof I think the first patch is enough to fix the warnings reported by @AkioNak, and the second patch (building a shared lib as pie) can actually be dropped here as it is a separate concern.

@AkioNak Could you try that and see if it works for you?

[AkioNak]

@theuni sure. I will try it soon.

[AkioNak]

@theuni @kallewoof fixed and rebased to master 727175a.
It works fine (there is no warnings about -pie, make check reports all PASS) on my enviroment , both macos10.13.4/XCode9.3 and ubuntu 17.10/g++7.2.0.

But Travis-Job #27212.2 and #27212.5 have failed at bitcoin-util-test.py.
#27212.2 https://travis-ci.org/bitcoin/bitcoin/jobs/364451508
#27212.5 https://travis-ci.org/bitcoin/bitcoin/jobs/364451511

[jonasschnelli]

Fixes my Xcode 9.3 maxOS 10.12 compiler warnings.
Tested ACK 6c1003589919b77dc9b143b4851cb123869b8fd0

[fanquake]

Also tested that 6c10035 fixes all -pie related warnings with macOS 10.13.4 and Xcode 9.3.

It seems like we don't have a clear conclusion on the second patch? We should open an issue to track that if required.

[theuni]

Thanks! utACK 6c1003589919b77dc9b143b4851cb123869b8fd0. Test failure seems unrelated, I'll restart it.

@fanquake Yes, it's not actually all that related here, since the new changes take care of the issue with libbitcoinconsensus also. And it's clearly not a real-world issue at the moment that we're building the library with -pie, as it seems to work fine. It'd be nice to find some real guidance, though, so that we at least know if it's just accidentally working.

[laanwj]

The travis failure in ../test/util/bitcoin-util-test.py is really strange (almost all testcases fail!), it didn't go away after cleaning the cache and restarting the buld.

Edit: see issue #12955

[theuni]

ok, seems this is a genuine problem. @AkioNak: Sorry that this got off-track.

checking whether C++ compiler accepts -fPIE... no
checking whether the linker accepts -pie... yes

A quick look around the net points out plenty of issues with mingw and pie.
Really, this combo shouldn't happen anyway. The GCC docs for -pie even warn about mismatches with:

For predictable results, you must also specify the same set of options used for compilation (-fpie, -fPIE, or model suboptions) when you specify this linker option.

I think we can eliminate the possibility with a combined check:

AX_CHECK_LINK_FLAG([[-fPIE -pie]], [PIE_FLAGS="-fPIE"; HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"],,[[$CXXFLAG_WERROR]])

[AkioNak]

@theuni thanks. fixed and rebased.
It seems to work fine with travis too.

[theuni]

Thanks. utACK.

[fanquake]

macOS 10.13.4
Xcode 9.3
Apple LLVM version 9.1.0 (clang-902.0.39.1)

Compiling master (e76acf3):

checking whether C++ compiler accepts -fPIE... yes
checking whether the linker accepts -pie... yes

clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
  CXX      qt/bitcoin_qt-bitcoin.o
  OBJCXX   qt/bitcoin_qt-macdockiconhandler.o
  OBJCXX   qt/bitcoin_qt-macnotificationhandler.o
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]

............

  CXXLD    bitcoind
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
  CXXLD    test/test_bitcoin
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
  CXXLD    bench/bench_bitcoin
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
  AR       qt/libbitcoinqt.a
  OBJCXXLD qt/bitcoin-qt
  CXXLD    qt/test/test_bitcoin-qt
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]
clang: warning: argument unused during compilation: '-pie' [-Wunused-command-line-argument]

Compiling (2eb5036):

checking whether the linker accepts -fPIE -pie... no

No -pie related compilation warnings

[jonasschnelli]

Tested ACK 2eb5036