[PATCH] D132725: [llvm][CodeGen] Skip WinCFGuard on non-Windows targets

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:588-589
   // Emit tables for any value of cfguard flag (i.e. cfguard=1 or cfguard=2).
-  if (mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("cfguard")))
+  if (Target.isOSWindows() &&
+      mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("cfguard")))
     Handlers.emplace_back(std::make_unique<WinCFGuard>(this), CFGuardName,
----------------
rnk wrote:
> aaron.ballman wrote:
> > CodeGen is not my area of strength, so this might be a silly question, but wouldn't it be better to prevent `cfguard` from being added to the module flags in the first place?
> I believe cfguard is an Xclang flag, so this is already protected in some ways. We generally add logic to the driver to defend against misusage, but the cc1 interface is relatively raw and unchecked.
> 
> So, I guess in conclusion, I think we probably have enough checking, and it makes sense for us to add this check to guard against crashing (assuming that's what happens later).
> 
> nit: make the condition `isOSBinFormatCOFF` just to guard against some obscure Windows ELF configurations.
SGTM, though I don't really want us to go overboard protecting against `-Xclang` misuse (when someone does that and gets a crash, I don't lose a whole lot of sleep because `-Xclang` voids all warranties).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132725