[PATCH] D77370: [libunwind] Add LIBUNWIND_ENABLE_PIC

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added a comment.

In D77370#1969771 <https://reviews.llvm.org/D77370#1969771>, @ldionne wrote:

> Ok, let's go step by step to make sure we don't talk past each other. Here's some facts (feel free to correct me if you see a mistake):
>
> 1. CMake builds **shared libraries** with `POSITION_INDEPENDENT_CODE=ON` by default.
> 2. CMake builds **static libraries** with `POSITION_INDEPENDENT_CODE=OFF` by default.
> 3. Setting `CMAKE_POSITION_INDEPENDENT_CODE=ON` on the command-line or in a cache will cause CMake to build both **shared libraries** and **static libraries** with `POSITION_INDEPENDENT_CODE=ON`.


@ldionne - I think that the facts you list are correct.

> IIUC, the problem we're trying to solve is that we want to build the **static libunwind library** with `POSITION_INDEPENDENT_CODE=ON`. Given that, why do we need a patch at all? Why isn't it sufficient to set `CMAKE_POSITION_INDEPENDENT_CODE=ON` in the Cache of whoever wants that behaviour?

Hmm, then I misunderstood the intent.  In my mind, the goal was to improve the default behaviour such that the static library is usable without having to explicitly specify it.  The linker should be able to strength reduce the static library relocations, so there is no cost associated (beyond intermediate code size) for doing that.

> While I think it would have been a better choice for the CMake designers to use `POSITION_INDEPENDENT_CODE=ON` by default for static libraries, I don't think it's wise to change that default on a per-project basis. This leads to surprises for anyone that assumes we're using CMake the way it was intended to be used.

Actually, if we want to do that, I think that we should redo the build to remove `LIBUNWIND_ENABLE_SHARED`/`LIBUNWIND_ENABLE_STATIC` - they are supposed to be specified by the user as `BUILD_SHARED_LIBS=[YES|NO]` not as a single build as it is currently.  But, that is beyond the scope of this change and more about the philosophical approach that you are suggesting here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77370