[PATCH] D99078: [LLD] Implement /guard:[no]ehcont

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: lld/COFF/DriverUtils.cpp:104-111
     else if (arg.equals_lower("nolongjmp"))
-      config->guardCF = GuardCFLevel::NoLongJmp;
-    else if (arg.equals_lower("cf") || arg.equals_lower("longjmp"))
-      config->guardCF = GuardCFLevel::Full;
+      config->guardCF &= ~GuardCFLevel::LongJmp;
+    else if (arg.equals_lower("noehcont"))
+      config->guardCF &= ~GuardCFLevel::EHCont;
+    else if (arg.equals_lower("cf"))
+      config->guardCF = GuardCFLevel::CF;
+    else if (arg.equals_lower("longjmp"))
----------------
alvinhochun wrote:
> This changed the behaviour of `/guard:cf`. Before this change, `/guard:cf` is equivalent to `/guard:cf,longjmp` by default, but after this change one must explicitly pass `/guard:longjmp` to`lld-link for it to generate the guard long jump table.
> 
> This also does not match the behaviour of MSVC's link.exe. From my quick testing, `/guard:cf`, `/guard:longjmp` and `/guard:cf,longjmp` all do the same thing for it to enable both CFGuard and the long jump table. Only when you pass `/guard:cf,nolongjmp` is the jump table not present in the final image.
Thanks for raising the problem. I don't find any description about the `longjmp` option in official document. Unless there's explicit document about the relationship between `/guard:cf`, `/guard:longjmp` and `/guard:cf,longjmp`, I don't think we need to match the exact behavior with MSVC. The single `/guard:cf` has ambiguity since we are supporting `ehcont`. So I'm not sure if the current MSVC behavior is constant in future versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99078