[PATCH] D132901: [LLD] Imply "longjmp" in `/guard:cf`

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/DriverUtils.cpp:109-110
       config->guardCF |= GuardCFLevel::CF | GuardCFLevel::LongJmp;
     else if (arg.equals_insensitive("ehcont"))
       config->guardCF |= GuardCFLevel::CF | GuardCFLevel::EHCont;
     else
----------------
pengfei wrote:
> alvinhochun wrote:
> > Since `ehcont` implies `cf`, perhaps in this case it should also imply `longjmp`?
> No idea. But if we make `ehcont` imply `longjmp`, an option like `-guard:cf,nolongjmp,ehcont` won't disable `nolongjmp`. So it should align with MSVC too. Could you help to have a check? Thanks!
I think to align with MSVC we'd have to change the code structure to process negation options after positive options, rather than updating the feature mask in order. We would also need to check the semantics of `/guard:longjump,nolonjump` / `/guard:nolongump,longjump`, those probably become warnings or errors.

Attention to detail like this is appreciated, but I'd also approve this change as is to get us back to where we were.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132901