[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:3929
+      options::OPT_fno_asynchronous_unwind_tables,
+      (TC.IsUnwindTablesDefault(Args) ||
+       (ExceptionArg &&
----------------
rnk wrote:
> Can you put this logic in the Windows or mingw toolchain instead by overriding IsUnwindTablesDefault? -fseh-exceptions feels fairly target specific.
> 
> If not, this boolean default feels fairly complicated. Here's how I would restructure it:
> ```
> bool UnwindTables = false;
> if (!Freestanding)
>   UnwindTables = TC.IsUnwindTablesDefault(Args) || TC.getSanitizerArgs().needsUnwindTables();
> UnwindTables = Args.hasFlag(OPT_fasync..., OPT_fno_async..., UnwindTables);
> UnwindTables = Args.hasFlag(OPT_munwind_tables, OPT_mno_unwind_tables, UnwindTables);
> ```
> Basically, we have some defaults dictated by the toolchain, and then two boolean flags, where one takes precedence over the other. -fno-async by itself will disable unwind tables, but `-munwind-tables -fno-asynchronous-unwind-tables` will leave them enabled, despite not following the usual "last flag wins" logic.
> Can you put this logic in the Windows or mingw toolchain instead by overriding IsUnwindTablesDefault? -fseh-exceptions feels fairly target specific.

Yes, that seems to be doable. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55749