ro added inline comments.
================ Comment at: clang/tools/driver/CMakeLists.txt:123 + +check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS) ---------------- MaskRay wrote: > ro wrote: > > MaskRay wrote: > > > ro wrote: > > > > MaskRay wrote: > > > > > GNU ld reports a warning instead of an error when an unknown `-z` is > > > > > seen. The warning remains a warning even with `--fatal-warnings`. > > > > > GNU ld reports a warning instead of an error when an unknown `-z` is > > > > > seen. The warning remains a warning even with `--fatal-warnings`. > > > > > > > > Thanks for reminding me about that misfeature of GNU `ld`. I guess > > > > `check_linker_flags` needs to be updated to handle that. > > > > In the case at hand, it won't matter either way: the flag is only > > > > passed to `ld`, which on Solaris is guaranteed to be the native linker. > > > > Once (if at all) I get around to completing D85309, I can deal with > > > > that. For now, other targets won't see linker warnings about this > > > > flag, other than when the flag is used at build time. > > > OK. Then I guess you can condition this when the OS is Solaris? > > > OK. Then I guess you can condition this when the OS is Solaris? > > > > I fear not: `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` is tested inside an `if` in > > `Solaris.cpp`: this code is also compiled on non-Solaris hosts. Why are > > you worried about the definition being always present? > It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong result > for GNU ld, even if it is not used for non-Solaris. We should make the value > correct in other configurations. > It is not suitable if LINKER_SUPPORTS_Z_RELAX_TRANSTLS returns a wrong result > for GNU ld, even if it is not used for non-Solaris. We should make the value > correct in other configurations. Tell the binutils maintainers that ;-) While I'm still unconcerned about this particular case (it's only used on a Solaris host where `clang` hardcodes the use of `/usr/bin/ld`), I continue to be annoyed by GNU `ld`'s nonsensical (or even outright dangerous) behaviour of accepting every `-z` option. It took me some wrestling with `cmake` , but now `check_linker_flag` correctly rejects `-z ` flags where GNU `ld` produces the warning. Some caveats about the implementation: - `check_cxx_compiler_flag` doesn't support the `FAIL_REGEX` arg, so I had to switch to `check_cxx_source_compiles` instead. - While it would be more appropriate to add the linker flag under test to `CMAKE_REQUIRED_LINK_OPTIONS`, that is only present since `cmake` 3.14 while LLVM still only requires 3.13. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91605/new/ https://reviews.llvm.org/D91605 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits