-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
cmake: picky-linker fixes for openssl, ZLIB, H3 and more #10857
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
not against this but should we be making concessions for these users that are compiling static builds? things can always break as the static library changes dependencies. for example what if the reporter did not enable zlib for curl, then isn't the problem the same, his build will fail because his ssl still requires zlib but cmake does not know that? shouldn't the burden be on him to specify the requirements if there's no |
@jay: It'd definitely help to state that. Static builds are extremely fragile and labour-intensive, curl-for-win being the living proof for that. An infinite amount of time can be spent tracking the variations and their changes. E.g. BoringSSL still needs On Windows, I think it'd also help if we added the base set of Windows system libs that are almost always required. They are a common source of problems there. |
That makes it easier to enable certain components by fixing detection logic. Ref: curl/curl#10857
35e7e44
to
7112c96
Compare
I am in favor of clearly documenting that doing working completely static curl builds is not the responsibility of the curl build scripts. To set the expectations right. |
Here's one reply (and some more below that), to a static linking report/question received earlier: curl/curl-for-win#39 (comment) (This was Windows-specific, which is a particularly problematic platform for this, but some of it is likely common to all.) And an unresolved discussion with a similar request: curl/curl-for-win#41 These were for final apps linking libcurl, which is kind of the same as linking curl itself. |
in mingw-w64 I used to do it like below for autotools. i don't know if something similar is possible with cmake. ./buildconf
CPPFLAGS="-DNGHTTP2_STATICLIB" LDFLAGS="-static" PKG_CONFIG="pkg-config --static" ./configure ...
make LDFLAGS="-static -all-static" V=1 then tests make test LDFLAGS="-static -all-static" V=1 then i would actually run the tests from msys2 shell instead of mingw-w64 shell for some reason. it's been a while since i've built like that. |
@jay: Thanks, I did not know about the |
- fix HTTP/3 support detection with openssl/quictls built with ZLIB - fix HTTP/3 support detection with openssl/quictls/libressl and `ld` linker on Windows - reposition ZLIB (and other compression) detection _after_ TLS detection, but before calling HTTP/3-support detection via `CheckQuicSupportInOpenSSL`. This seems to fix an odd case, where OpenSSL (quictls) is correctly detected, but its header path is not set while compiling, breaking it at `src/curl_ntlm_core.c`. Regression from ebef55a May fix curl#10832
7112c96
to
217cfda
Compare
- fix HTTP/3 support detection with OpenSSL/quictls built with ZLIB. (Requires curl be built with ZLIB option also.) - fix HTTP/3 support detection with OpenSSL/quictls/LibreSSL and `ld` linker on Windows. - fix HTTP/3 support detection with wolfSSL to automatically add `ws2_32` to the lib list on Windows. For all linkers. - reposition ZLIB (and other compression) detection _after_ TLS detection, but before calling HTTP/3-support detection via `CheckQuicSupportInOpenSSL`. May be a regression from ebef55a May fix curl#10832 (Reported-by: Micah Snyder) This also seems to fix an odd case, where OpenSSL/quictls is correctly detected, but its header path is not set while compiling, breaking build at `src/curl_ntlm_core.c`. Reason for this remains undiscovered. - satisfy "picky" linkers such as `ld` with MinGW, that are highly sensitive to lib order, by also adding brotli to the beginning of the lib list. - satisfy "picky" linkers by adding certain Windows systems libs to the lib list for OpenSSL/LibreSSL. (Might need additional ones for other forks, such as `pthread` for BoringSSL.) Note: It'd make sense to _always_ add `ws2_32`, `crypt32` (except Windows App targets perhaps?), `bcrypt` (except old-mingw!) on Windows at this point. They are almost always required, and if some aren't, they are ignored by the linker with no effect on final binaries. Closes curl#10857
list(PREPEND CURL_LIBS ${BROTLI_LIBRARIES}) | ||
list(APPEND CURL_LIBS ${BROTLI_LIBRARIES}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt it ever made sense to PREPEND in addition to APPEND.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been a while ago, and can't recall the exact test results for this specific case. But, it's unlikely I'd add such thing unless required to fix the picky-linker problem. (A clean solution would be -Wl,--start-group
/ -Wl,--end-group
, but could not figure out how to apply that to CMake.)
If a better solution to this exists, I'd be very glad to learn about it. It's been a two-decade problem with this linker and static linking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "picky-linker problem" is unknown to me, but I know that the order of static libs matters outside MSVC.
And I know that CMake may mess with the order unless it is properly defined by target dependencies. Subject to CMake version.
(And I know that meson still sorts link libs it gets from CMake...)
FTR I'm about to patch out the PREPEND line in the vcpkg port. Vcpkg CI builds with static library linkage for windows, linux, osx, and android. And of course I'm just tracing a linking issue. I want to get rid of distractors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Picky-linker" is my term, it applies to binutils ld
specifically. It may apply to others, but I haven't seen it anywhere else. Also llvm/clang's lld
doesn't have this problem, neither does MSVC. My understanding is that the linker does no effort to resolve symbols in a second pass, so if a symbol is referenced earlier than defined, linking breaks. It must have a strong reason doing this for decades and without a fix.
Feel free to patch it out locally, though I'm not aware of any issues it may cause. You may also move it inside an if(CMAKE_COMPILER_IS_GNUCC)
, if it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the linker does no effort to resolve symbols in a second pass, so if a symbol is referenced earlier than defined, linking breaks.
It is the other way round. First the dependent lib (-lbrotlidec
), then the independent lib (-lbrotlicommon
).
If you get it wrong, then you need to repeat it.
And that's why extra prepending is still in the "never made sense" box for me.
(Hint: -lz
and -lm
are typically found at the end of the link libs.)
And that's why "sort" and "remove duplicates" do not work on link libs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this help with the root cause for brotli?:
diff --git a/CMake/FindBrotli.cmake b/CMake/FindBrotli.cmake
index 11ab7f825..dcab8f7cd 100644
--- a/CMake/FindBrotli.cmake
+++ b/CMake/FindBrotli.cmake
@@ -40,4 +40,4 @@ find_package_handle_standard_args(Brotli
)
set(BROTLI_INCLUDE_DIRS ${BROTLI_INCLUDE_DIR})
-set(BROTLI_LIBRARIES ${BROTLICOMMON_LIBRARY} ${BROTLIDEC_LIBRARY})
+set(BROTLI_LIBRARIES ${BROTLIDEC_LIBRARY} ${BROTLICOMMON_LIBRARY})
I'm trying to replicate it, but without knowing the exact trigger configuration, this might take some time. If it triggers in my current test setup at all.
edit: managed to replicate and confirm that the above fix works, without the prepend hack.
Fix the root cause that resulted in missing symbols when linking brotli statically with binutils `ld`. Also drop existing workaround that added brotli libs twice to the lib list. ``` /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.text$ProcessCommands[ProcessCommands]+0xbb5): undefined reference to `BrotliTransformDictionaryWord' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.text$SafeProcessCommands[SafeProcessCommands]+0xe8a): undefined reference to `BrotliTransformDictionaryWord' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.rdata$.refptr._kBrotliContextLookupTable[.refptr._kBrotliContextLookupTable]+0x0): undefined reference to `_kBrotliContextLookupTable' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.rdata$.refptr._kBrotliPrefixCodeRanges[.refptr._kBrotliPrefixCodeRanges]+0x0): undefined reference to `_kBrotliPrefixCodeRanges' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x21): undefined reference to `BrotliDefaultAllocFunc' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x2f): undefined reference to `BrotliDefaultFreeFunc' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x10e): undefined reference to `BrotliSharedDictionaryCreateInstance' /usr/local/Cellar/mingw-w64/11.0.1_1/toolchain-x86_64/bin/x86_64-w64-mingw32-ld: .../curl/brotli/_x64-win-ucrt/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateCleanup[BrotliDecoderStateCleanup]+0xf4): undefined reference to `BrotliSharedDictionaryDestroyInstance' collect2: error: ld returned 1 exit status ``` Breakage reproducible with curl-for-win config `gcc' and this patch: ```diff --- a/curl.sh +++ b/curl.sh @@ -65,14 +65,6 @@ _VER="$1" fi fi - # Ugly hack. Everything breaks without this due to the accidental ordering - # of libs and objects, and offering no universal way to (re)insert libs at - # specific positions. Linker complains about a missing --end-group, then - # adds it automatically anyway. - if [ "${_LD}" = 'ld' ]; then - LDFLAGS+=' -Wl,--start-group' - fi - if [ "${_OS}" = 'win' ]; then # Link lib dependencies in static mode. Implied by `-static` for curl, # but required for libcurl, which would link to shared libs by default. ``` (curl-for-win hack still required for some non-brotli cases.) Assisted-by: Kai Pastor Ref: curl#10857 (comment) Follow-up to 1e3319a curl#10857 Closes #xxxxx
Fix root cause that caused missing symbols when linking brotli statically with e.g. binutils `ld` (and any other "picky" linker, or "traditional" linker as CMake now calls them). Also drop existing workaround that added brotli libs twice to the lib list. ``` x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.text$ProcessCommands[ProcessCommands]+0xbb5): undefined reference to `BrotliTransformDictionaryWord' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.text$SafeProcessCommands[SafeProcessCommands]+0xe8a): undefined reference to `BrotliTransformDictionaryWord' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.rdata$.refptr._kBrotliContextLookupTable[.refptr._kBrotliContextLookupTable]+0x0): undefined reference to `_kBrotliContextLookupTable' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(decode.c.obj):decode.c:(.rdata$.refptr._kBrotliPrefixCodeRanges[.refptr._kBrotliPrefixCodeRanges]+0x0): undefined reference to `_kBrotliPrefixCodeRanges' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x21): undefined reference to `BrotliDefaultAllocFunc' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x2f): undefined reference to `BrotliDefaultFreeFunc' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateInit[BrotliDecoderStateInit]+0x10e): undefined reference to `BrotliSharedDictionaryCreateInstance' x86_64-w64-mingw32-ld: .../curl/brotli/_bld/usr/lib/libbrotlidec.a(state.c.obj):state.c:(.text$BrotliDecoderStateCleanup[BrotliDecoderStateCleanup]+0xf4): undefined reference to `BrotliSharedDictionaryDestroyInstance' collect2: error: ld returned 1 exit status ``` Breakage reproducible with curl-for-win config "`win-gcc`" and deleting the `LDFLAGS+=' -Wl,--start-group'` line from its `curl.sh` script. (Above line still required for some non-brotli cases, e.g. libssh2 and zlib.) Assisted-by: Kai Pastor Ref: #10857 (comment) Follow-up to 1e3319a #10857 Closes #13761
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
1e3319a curl#10857 Double-check for no regressions: ebef55a curl#10832 An ideal detection order would be this: zlib/brotli/zstd libuv cares libpsl libidn2 librtmp nghttp3 nettle tls-backends bearssl/gnutls/openssl/mbedtls/wolfssl/msh3/quiche/rustls ngtcp2 nghttp2 gsasl/gss/ldap libssh/libssh2/wolfssh While always prepending detected libs to the liblist.
fix HTTP/3 support detection with OpenSSL/quictls built with ZLIB.
(Requires curl be built with ZLIB option also.)
fix HTTP/3 support detection with OpenSSL/quictls/LibreSSL and
ld
linker on Windows.
fix HTTP/3 support detection with wolfSSL to automatically add
ws2_32
to the lib list on Windows. For all linkers.reposition ZLIB (and other compression) detection after TLS
detection, but before calling HTTP/3-support detection via
CheckQuicSupportInOpenSSL
.May be a regression from ebef55a
May fix undefined reference for zlib symbols when building with static dependencies and BUILD_SHARED_LIBS=OFF #10832 (Reported-by: Micah Snyder)
This also seems to fix an odd case, where OpenSSL/quictls is correctly
detected, but its header path is not set while compiling, breaking
build at
src/curl_ntlm_core.c
. Reason for this remains undiscovered.satisfy "picky" linkers such as
ld
with MinGW, that are highlysensitive to lib order, by also adding brotli to the beginning of the
lib list.
satisfy "picky" linkers by adding certain Windows systems libs to
the lib list for OpenSSL/LibreSSL. (Might need additional ones for
other forks, such as
pthread
for BoringSSL.)Note: It'd make sense to always add
ws2_32
,crypt32
(exceptWindows App targets perhaps?),
bcrypt
(except old-mingw!) on Windowsat this point. They are almost always required, and if some aren't,
they are ignored by the linker with no effect on final binaries.
Closes #10857
With these, a CMake build with gcc/ld almost works ;) Now→ Worked around usinggsasl
lib is in the wrong position ¯\(ツ)/¯.CMAKE_C_STANDARD_LIBRARIES
to pass it.