[libcxx-commits] [PATCH] D71117: Reland "Enable `-funwind-tables` flag when building libunwind"

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

In D71117#1774395 <https://reviews.llvm.org/D71117#1774395>, @broadwaylamb wrote:

> @mstorsjo thanks for verifying!
>
> When preparing this patch I've looked through the flags being added and couldn't find those that would benefit from this.
>
> 1. Obviously, this won't affect any `-W` flags.
> 2. Same for `-fstrict-aliasing`.
> 3. `-EHsc` is MSVC-specific and won't generate any new calls (will it?).
> 4. `-fno-exceptions` and `-fno-rtti` won't add any new calls either.
> 5. Same for `-nostdinc++`.
> 6. `-D` and `-U` are only for the preprocessor.
>
>   So, it seems like here we only want `-funwind-tables` to be guarded with this CMake setting.
>
> > but it still might be good for consistency in any case?
>
> Now that you've demonstrated the issues that this flag might uncover, my opinion is we probably only want this flag to be set in places we know for sure it is needed, with a clear explanation why it is needed.


Well I guess it's whether you aim to be as safe as possible from this kind of issue, or consistency/simplicity with broader groupings. I'd say in general this flag should be rather safe for anything which is a test for a compiler feature (instead of trying to dissect exactly which compiler options might add symbol references), but clearly not for tests for linker features.

But I won't insist, if you prefer keeping it this narrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71117