nickdesaulniers created this revision.
nickdesaulniers added reviewers: void, rnk, phosek, srhines.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nickdesaulniers requested review of this revision.

Make the virtual method Toolchain::GetDefaultStackProtectorLevel()
return an explict enum value rather than an integral constant. This
makes the code subjectively easier to read, and should help prevent bugs
that may (or may never) arise from changing the enum values. Previously,
these were just kept in sync via a comment, which is brittle. The trade
off is including a additional header in a few new places. It is not
necessary, but in my opinion helps the readability.

Split off from https://reviews.llvm.org/D90194 to help cut down on lines
changed in code review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90271

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CrossWindows.h
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/OpenBSD.h
  clang/lib/Driver/ToolChains/PS4CPU.h

Index: clang/lib/Driver/ToolChains/PS4CPU.h
===================================================================
--- clang/lib/Driver/ToolChains/PS4CPU.h
+++ clang/lib/Driver/ToolChains/PS4CPU.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_PS4CPU_H
 
 #include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -73,8 +74,9 @@
   bool HasNativeLLVMSupport() const override;
   bool isPICDefault() const override;
 
-  unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
-    return 2; // SSPStrong
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+    return LangOptions::SSPStrong;
   }
 
   llvm::DebuggerKind getDefaultDebuggerTuning() const override {
Index: clang/lib/Driver/ToolChains/OpenBSD.h
===================================================================
--- clang/lib/Driver/ToolChains/OpenBSD.h
+++ clang/lib/Driver/ToolChains/OpenBSD.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_OPENBSD_H
 
 #include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -79,8 +80,9 @@
   std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component,
                             FileType Type = ToolChain::FT_Static) const override;
 
-  unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
-    return 2;
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+    return LangOptions::SSPStrong;
   }
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===================================================================
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_FUCHSIA_H
 
 #include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -59,8 +60,9 @@
     return llvm::DebuggerKind::GDB;
   }
 
-  unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
-    return 2; // SSPStrong
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+    return LangOptions::SSPStrong;
   }
 
   std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/Darwin.h
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -11,6 +11,7 @@
 
 #include "Cuda.h"
 #include "ROCm.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Driver/DarwinSDKInfo.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
@@ -491,17 +492,18 @@
     return !(isTargetMacOS() && isMacosxVersionLT(10, 6));
   }
 
-  unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
     // Stack protectors default to on for user code on 10.5,
     // and for everything in 10.6 and beyond
     if (isTargetIOSBased() || isTargetWatchOSBased())
-      return 1;
+      return LangOptions::SSPOn;
     else if (isTargetMacOS() && !isMacosxVersionLT(10, 6))
-      return 1;
+      return LangOptions::SSPOn;
     else if (isTargetMacOS() && !isMacosxVersionLT(10, 5) && !KernelOrKext)
-      return 1;
+      return LangOptions::SSPOn;
 
-    return 0;
+    return LangOptions::SSPOff;
   }
 
   void CheckObjCARC() const override;
Index: clang/lib/Driver/ToolChains/CrossWindows.h
===================================================================
--- clang/lib/Driver/ToolChains/CrossWindows.h
+++ clang/lib/Driver/ToolChains/CrossWindows.h
@@ -11,6 +11,7 @@
 
 #include "Cuda.h"
 #include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 
@@ -59,8 +60,9 @@
   bool isPIEDefault() const override;
   bool isPICDefaultForced() const override;
 
-  unsigned int GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
-    return 0;
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+    return LangOptions::SSPOff;
   }
 
   void
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2995,8 +2995,8 @@
     return;
 
   // -stack-protector=0 is default.
-  unsigned StackProtectorLevel = 0;
-  unsigned DefaultStackProtectorLevel =
+  LangOptions::StackProtectorMode StackProtectorLevel = LangOptions::SSPOff;
+  LangOptions::StackProtectorMode DefaultStackProtectorLevel =
       TC.GetDefaultStackProtectorLevel(KernelOrKext);
 
   if (Arg *A = Args.getLastArg(options::OPT_fno_stack_protector,
@@ -3005,7 +3005,7 @@
                                options::OPT_fstack_protector)) {
     if (A->getOption().matches(options::OPT_fstack_protector))
       StackProtectorLevel =
-          std::max<unsigned>(LangOptions::SSPOn, DefaultStackProtectorLevel);
+          std::max<>(LangOptions::SSPOn, DefaultStackProtectorLevel);
     else if (A->getOption().matches(options::OPT_fstack_protector_strong))
       StackProtectorLevel = LangOptions::SSPStrong;
     else if (A->getOption().matches(options::OPT_fstack_protector_all))
Index: clang/include/clang/Driver/ToolChain.h
===================================================================
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -382,9 +382,10 @@
   virtual bool useRelaxRelocations() const;
 
   /// GetDefaultStackProtectorLevel - Get the default stack protector level for
-  /// this tool chain (0=off, 1=on, 2=strong, 3=all).
-  virtual unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const {
-    return 0;
+  /// this tool chain.
+  virtual LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const {
+    return LangOptions::SSPOff;
   }
 
   /// Get the default trivial automatic variable initialization.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to