macOS: Prevent Xcode 9.3 build warnings #12899

pull AkioNak wants to merge 1 commits into bitcoin:master from AkioNak:preventxcodebuildwarnings changing 1 files +1 −5
  1. AkioNak commented at 5:08 AM on April 6, 2018: contributor

    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.
  2. fanquake added the label macOS on Apr 6, 2018
  3. kallewoof commented at 5:23 AM on April 6, 2018: member

    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.

  4. fanquake added the label Build system on Apr 6, 2018
  5. fanquake requested review from theuni on Apr 6, 2018
  6. l2a5b1 commented at 11:49 PM on April 6, 2018: contributor

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

  7. in configure.ac:636 in 12ebae7ba7 outdated 
     632 | @@ -633,7 +633,7 @@ if test x$use_hardening != xno; then
 633 |  
 634 |    if test x$TARGET_OS != xwindows; then
 635 |      AX_CHECK_COMPILE_FLAG([-fPIE],[PIE_FLAGS="-fPIE"])
 636 | -    AX_CHECK_LINK_FLAG([[-pie]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"])
 637 | +    AX_CHECK_LINK_FLAG([[-Wl,-pie]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-pie"])
    laanwj commented at 2:27 PM on April 9, 2018:

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

  8. theuni commented at 5:39 PM on April 9, 2018: member

    (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.

  9. AkioNak commented at 10:01 PM on April 9, 2018: contributor

    @theuni Thank you for your review.

  10. kallewoof commented at 1:57 AM on April 10, 2018: member

    @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.

  11. AkioNak commented at 2:05 AM on April 10, 2018: contributor

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

  12. theuni commented at 2:07 AM on April 10, 2018: member

    @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?

  13. AkioNak commented at 2:10 AM on April 10, 2018: contributor

    @theuni sure. I will try it soon.

  14. AkioNak force-pushed on Apr 10, 2018
  15. AkioNak commented at 8:01 AM on April 10, 2018: contributor

    @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

  16. jonasschnelli commented at 8:19 AM on April 11, 2018: contributor

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

  17. fanquake commented at 3:30 PM on April 11, 2018: member

    Also tested that https://github.com/bitcoin/bitcoin/commit/6c1003589919b77dc9b143b4851cb123869b8fd0 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.

  18. theuni commented at 6:51 PM on April 11, 2018: member

    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.

  19. laanwj commented at 10:18 AM on April 12, 2018: member

    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

  20. theuni commented at 6:14 PM on April 12, 2018: member

    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]])
  21. macOS: Prevent Xcode 9.3 build warnings
    This PR solves #12867 (needs to run autogen.sh && ./configure)

clang (Apple LLVM version 9.1.0 (clang-902.0.39.1)) warns unused
argument '-pie' during compilation.
So we check for warnings in the test using $CXXFLAG_WERROR.

Windows is alse default-pie and was special-cased because it also
warned, but we can also eliminate that case if warnings are caught.
    2eb5036c33
  22. AkioNak force-pushed on Apr 13, 2018
  23. AkioNak commented at 3:43 AM on April 13, 2018: contributor

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

  24. theuni commented at 11:36 PM on April 13, 2018: member

    Thanks. utACK.

  25. fanquake commented at 2:58 AM on April 16, 2018: member

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

    Compiling master (e76acf3384aca13cd2dd850568317c0ce44458ed):

    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 (https://github.com/bitcoin/bitcoin/pull/12899/commits/2eb5036c330fcc4fc115fe378b8211fd14711195):

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

    No -pie related compilation warnings

  26. jonasschnelli commented at 8:58 AM on April 17, 2018: contributor

    Tested ACK 2eb5036c330fcc4fc115fe378b8211fd14711195

  27. jonasschnelli merged this on Apr 17, 2018
  28. jonasschnelli closed this on Apr 17, 2018
  29. jonasschnelli referenced this in commit 3076993048 on Apr 17, 2018
  30. AkioNak deleted the branch on Apr 17, 2018
  31. PastaPastaPasta referenced this in commit cde4278a6a on Jun 17, 2020
  32. PastaPastaPasta referenced this in commit e6c2b02f44 on Jun 27, 2020
  33. gades referenced this in commit f9fea45bb5 on Jun 26, 2021
  34. MarcoFalke locked this on Sep 8, 2021
Contributors
AkioNakkallewoofl2a5b1laanwjtheunijonasschnellifanquake


theuni

Labels
macOSBuild system
Linked (view graph)
#12867 macOS: Xcode 9.3 build warnings#12955 travis: Windows build failing after -pie changes
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me