[libcxx-commits] [PATCH] D113253: [runtimes] Fix building initial libunwind+libcxxabi+libcxx with compiler implied -lunwind

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.


================
Comment at: runtimes/CMakeLists.txt:101-102
+
+check_c_compiler_flag("" LLVM_RUNTIMES_LINKING_WORKS)
+if (NOT LLVM_RUNTIMES_LINKING_WORKS)
+  # The compiler driver may be implicitly trying to link against libunwind, which
----------------
mstorsjo wrote:
> ldionne wrote:
> > mstorsjo wrote:
> > > ldionne wrote:
> > > > I don't understand how this check works. Since you are disabling runtime linking, I would think that `check_c_compiler_flag` succeeding does NOT imply that runtime linking works?
> > > We don’t disable linking here - this is cmake’s standard compile+link test. We do one such test, adding no flags, to see if linking works out of the box.
> > > 
> > > If it succeeds, we have a complete toolchain and don’t need to do anything. If linking doesn’t seem to work, it could be an incomplete toolchain being built - in those cases, check if we could make it work by adding `--unwindlib=none`.
> > > 
> > > The reason for this indirection is that if we have a complete toolchain, adding that option can break things - if we’re setting up building with sanitizers (as in CI), adding that option breaks the sanitizers.
> > Don't you disable runtime linking in compiler checks on line 18 above?
> > 
> > ```
> > include(EnableLanguageNolink)
> > project(Runtimes LANGUAGES NONE)
> > llvm_enable_language_nolink(C CXX ASM)
> > ```
> No, that macro just disables linking temporarily while running the basic compiler sanity checks. We do need to have working linker tests for some later things - but it’s quite fiddly when the toolchain is incomplete.
> 
> Temporarily disabling linking in the sanity checks so the tests succeed allows cmake to pick up general features about the target (OS etc). And if not disabled, one has to pass `-DCMAKE_C_COMPILER_WORKS=YES` to waive the checks.
Ah, I re-read the definition of `llvm_enable_language_nolink` and it all became very obvious. Sorry for the naive questions, and thanks for explaining. 🚢 it!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113253/new/

https://reviews.llvm.org/D113253