[clang] Move branch-protection from CodeGenOptions to LangOptions

[tamas.petz]

Summary

Reason: the option has an effect on preprocessing.

Also see thread: http://lists.llvm.org/pipermail/cfe-dev/2020-March/065014.html

[danielkiss]

just a nit, LGTM otherwise.

clang/include/clang/Basic/CodeGenOptions.def

402

is this change necessary?

[tamas.petz]

clang/include/clang/Basic/CodeGenOptions.def

402

I think there should be one new line at the end of the file, not two.

[tamas.petz]

Tests pass, however, I am not sure ParseLangArgs is called in all the necessary cases (see the occurrences in the code).

[chill]

Following @efriedma comment here http://lists.llvm.org/pipermail/cfe-dev/2020-March/065017.html LGTM.

clang/include/clang/Basic/TargetInfo.h

19

This include is probably not needed anymore.

[tamas.petz]

Address review comments.

[efriedma]

LGTM

clang/include/clang/Basic/CodeGenOptions.h

public:
 
   enum EmbedBitcodeKind {
     Embed_Off,      // No embedded bitcode.
     Embed_All,      // Embed both bitcode and commandline in the output.
     Embed_Bitcode,  // Embed just the bitcode in the output.
     Embed_Marker    // Embed a marker as a placeholder for bitcode.
   };
 
-  enum class SignReturnAddressScope {
-    None,    // No signing for any function
-    NonLeaf, // Sign the return address of functions that spill LR
-    All      // Sign the return address of all functions
-  };
-
-  enum class SignReturnAddressKeyValue { AKey, BKey };
-
   enum class FramePointerKind {
     None,        // Omit all frame pointers.
     NonLeaf,     // Keep non-leaf frame pointers.
     All,         // Keep all frame pointers.
   };
 
   /// The code model to use (-mcmodel).
   std::string CodeModel;

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(EmbedSource, 1, 0)
 
 /// Whether to emit all vtables
 CODEGENOPT(ForceEmitVTables, 1, 0)
 
 /// Whether to emit an address-significance table into the object file.
 CODEGENOPT(Addrsig, 1, 0)
 
-ENUM_CODEGENOPT(SignReturnAddress, SignReturnAddressScope, 2, SignReturnAddressScope::None)
-ENUM_CODEGENOPT(SignReturnAddressKey, SignReturnAddressKeyValue, 1, SignReturnAddressKeyValue::AKey)
-CODEGENOPT(BranchTargetEnforcement, 1, 0)
-
 /// Whether to emit unused static constants.
 CODEGENOPT(KeepStaticConsts, 1, 0)
 
 /// Whether to not follow the AAPCS that enforce at least one read before storing to a volatile bitfield
 CODEGENOPT(ForceAAPCSBitfieldLoad, 1, 0)
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
-

clang/include/clang/Basic/LangOptions.h

enum class LaxVectorConversionKind {
     /// Permit vector bitcasts between integer vectors with different numbers
     /// of elements but the same total bit-width.
     Integer,
     /// Permit vector bitcasts between all vectors with the same total
     /// bit-width.
     All,
   };
 
+  enum class SignReturnAddressScopeKind {
+    /// No signing for any function.
+    None,
+    /// Sign the return address of functions that spill LR.
+    NonLeaf,
+    /// Sign the return address of all functions,
+    All
+  };
+
+  enum class SignReturnAddressKeyKind {
+    /// Return address signing uses APIA key.
+    AKey,
+    /// Return address signing uses APIB key.
+    BKey
+  };
+
 public:
   /// Set of enabled sanitizers.
   SanitizerSet Sanitize;
 
   /// Paths to blacklist files specifying which objects
   /// (files, functions, variables) should not be instrumented.
   std::vector<std::string> SanitizerBlacklistFiles;
 

clang/include/clang/Basic/LangOptions.def

 LANGOPT(FixedPoint, 1, 0, "fixed point types")
 LANGOPT(PaddingOnUnsignedFixedPoint, 1, 0,
         "unsigned fixed point types having one extra padding bit")
 
 LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors")
 
 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0")
 
+ENUM_LANGOPT(SignReturnAddressScope, SignReturnAddressScopeKind, 2, SignReturnAddressScopeKind::None,
+             "Scope of return address signing")
+ENUM_LANGOPT(SignReturnAddressKey, SignReturnAddressKeyKind, 1, SignReturnAddressKeyKind::AKey,
+             "Key used for return address signing")
+LANGOPT(BranchTargetEnforcement, 1, 0, "Branch-target enforcement enabled")
+
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT
 #undef ENUM_LANGOPT
 #undef COMPATIBLE_ENUM_LANGOPT
 #undef BENIGN_ENUM_LANGOPT
 #undef VALUE_LANGOPT
 #undef COMPATIBLE_VALUE_LANGOPT
 #undef BENIGN_VALUE_LANGOPT

clang/include/clang/Basic/TargetInfo.h

 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_TARGETINFO_H
 #define LLVM_CLANG_BASIC_TARGETINFO_H
 
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/LLVM.h"
-#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetCXXABI.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
public:
   }
 
   /// Determine whether this TargetInfo supports the given feature.
   virtual bool isValidFeatureName(StringRef Feature) const {
     return true;
   }
 
   struct BranchProtectionInfo {
-    CodeGenOptions::SignReturnAddressScope SignReturnAddr =
-        CodeGenOptions::SignReturnAddressScope::None;
-    CodeGenOptions::SignReturnAddressKeyValue SignKey =
-        CodeGenOptions::SignReturnAddressKeyValue::AKey;
+    LangOptions::SignReturnAddressScopeKind SignReturnAddr =
+        LangOptions::SignReturnAddressScopeKind::None;
+    LangOptions::SignReturnAddressKeyKind SignKey =
+        LangOptions::SignReturnAddressKeyKind::AKey;
     bool BranchTargetEnforcement = false;
   };
 
   /// Determine if this TargetInfo supports the given branch protection
   /// specification
   virtual bool validateBranchProtection(StringRef Spec,
                                         BranchProtectionInfo &BPI,
                                         StringRef &Err) const {

clang/lib/AST/PrintfFormatString.cpp

 //== PrintfFormatString.cpp - Analysis of printf format strings --*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Handling of format string in printf and friends.  The structure of format
 // strings for fprintf() are described in C99 7.19.6.1.
 //
 //===----------------------------------------------------------------------===//
 
+#include "FormatStringParsing.h"
 #include "clang/AST/FormatString.h"
 #include "clang/AST/OSLog.h"
-#include "FormatStringParsing.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/Support/Regex.h"
 
 using clang::analyze_format_string::ArgType;
 using clang::analyze_format_string::FormatStringHandler;
 using clang::analyze_format_string::LengthModifier;
 using clang::analyze_format_string::OptionalAmount;
 using clang::analyze_format_string::ConversionSpecifier;
 using clang::analyze_printf::PrintfSpecifier;
 

clang/lib/Basic/Targets/AArch64.cpp

 //===--- AArch64.cpp - Implement AArch64 target feature support -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements AArch64 TargetInfo objects.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AArch64.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/AArch64TargetParser.h"
 
 using namespace clang;

 bool AArch64TargetInfo::validateBranchProtection(StringRef Spec,
                                                  BranchProtectionInfo &BPI,
                                                  StringRef &Err) const {
   llvm::AArch64::ParsedBranchProtection PBP;
   if (!llvm::AArch64::parseBranchProtection(Spec, PBP, Err))
     return false;
 
   BPI.SignReturnAddr =
-      llvm::StringSwitch<CodeGenOptions::SignReturnAddressScope>(PBP.Scope)
-          .Case("non-leaf", CodeGenOptions::SignReturnAddressScope::NonLeaf)
-          .Case("all", CodeGenOptions::SignReturnAddressScope::All)
-          .Default(CodeGenOptions::SignReturnAddressScope::None);
+      llvm::StringSwitch<LangOptions::SignReturnAddressScopeKind>(PBP.Scope)
+          .Case("non-leaf", LangOptions::SignReturnAddressScopeKind::NonLeaf)
+          .Case("all", LangOptions::SignReturnAddressScopeKind::All)
+          .Default(LangOptions::SignReturnAddressScopeKind::None);
 
   if (PBP.Key == "a_key")
-    BPI.SignKey = CodeGenOptions::SignReturnAddressKeyValue::AKey;
+    BPI.SignKey = LangOptions::SignReturnAddressKeyKind::AKey;
   else
-    BPI.SignKey = CodeGenOptions::SignReturnAddressKeyValue::BKey;
+    BPI.SignKey = LangOptions::SignReturnAddressKeyKind::BKey;
 
   BPI.BranchTargetEnforcement = PBP.BranchTargetEnforcement;
   return true;
 }
 
 bool AArch64TargetInfo::isValidCPUName(StringRef Name) const {
   return Name == "generic" ||
          llvm::AArch64::parseCPUArch(Name) != llvm::AArch64::ArchKind::INVALID;

clang/lib/CodeGen/CGDeclCXX.cpp

 //===----------------------------------------------------------------------===//
 
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenMPRuntime.h"
 #include "CodeGenFunction.h"
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
-#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/Path.h"
 
 using namespace clang;
 using namespace CodeGen;
 
llvm::Function *CodeGenModule::CreateGlobalInitOrDestructFunction(
   if (getLangOpts().Sanitize.has(SanitizerKind::SafeStack) &&
       !isInSanitizerBlacklist(SanitizerKind::SafeStack, Fn, Loc))
     Fn->addFnAttr(llvm::Attribute::SafeStack);
 
   if (getLangOpts().Sanitize.has(SanitizerKind::ShadowCallStack) &&
       !isInSanitizerBlacklist(SanitizerKind::ShadowCallStack, Fn, Loc))
     Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
-  auto RASignKind = getCodeGenOpts().getSignReturnAddress();
-  if (RASignKind != CodeGenOptions::SignReturnAddressScope::None) {
+  auto RASignKind = getLangOpts().getSignReturnAddressScope();
+  if (RASignKind != LangOptions::SignReturnAddressScopeKind::None) {
     Fn->addFnAttr("sign-return-address",
-                  RASignKind == CodeGenOptions::SignReturnAddressScope::All
+                  RASignKind == LangOptions::SignReturnAddressScopeKind::All
                       ? "all"
                       : "non-leaf");
-    auto RASignKey = getCodeGenOpts().getSignReturnAddressKey();
+    auto RASignKey = getLangOpts().getSignReturnAddressKey();
     Fn->addFnAttr("sign-return-address-key",
-                  RASignKey == CodeGenOptions::SignReturnAddressKeyValue::AKey
+                  RASignKey == LangOptions::SignReturnAddressKeyKind::AKey
                       ? "a_key"
                       : "b_key");
   }
 
-  if (getCodeGenOpts().BranchTargetEnforcement)
+  if (getLangOpts().BranchTargetEnforcement)
     Fn->addFnAttr("branch-target-enforcement");
 
   return Fn;
 }
 
 /// Create a global pointer to a function that will initialize a global
 /// variable.  The user has requested that this pointer be emitted in a specific
 /// section.

clang/lib/CodeGen/TargetInfo.cpp

public:
   bool doesReturnSlotInterfereWithArgs() const override { return false; }
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
                            CodeGen::CodeGenModule &CGM) const override {
     const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
     if (!FD)
       return;
 
-    CodeGenOptions::SignReturnAddressScope Scope = CGM.getCodeGenOpts().getSignReturnAddress();
-    CodeGenOptions::SignReturnAddressKeyValue Key = CGM.getCodeGenOpts().getSignReturnAddressKey();
-    bool BranchTargetEnforcement = CGM.getCodeGenOpts().BranchTargetEnforcement;
+    LangOptions::SignReturnAddressScopeKind Scope =
+        CGM.getLangOpts().getSignReturnAddressScope();
+    LangOptions::SignReturnAddressKeyKind Key =
+        CGM.getLangOpts().getSignReturnAddressKey();
+    bool BranchTargetEnforcement = CGM.getLangOpts().BranchTargetEnforcement;
     if (const auto *TA = FD->getAttr<TargetAttr>()) {
       ParsedTargetAttr Attr = TA->parse();
       if (!Attr.BranchProtection.empty()) {
         TargetInfo::BranchProtectionInfo BPI;
         StringRef Error;
         (void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
                                                        BPI, Error);
         assert(Error.empty());
         Scope = BPI.SignReturnAddr;
         Key = BPI.SignKey;
         BranchTargetEnforcement = BPI.BranchTargetEnforcement;
       }
     }
 
     auto *Fn = cast<llvm::Function>(GV);
-    if (Scope != CodeGenOptions::SignReturnAddressScope::None) {
+    if (Scope != LangOptions::SignReturnAddressScopeKind::None) {
       Fn->addFnAttr("sign-return-address",
-                    Scope == CodeGenOptions::SignReturnAddressScope::All
+                    Scope == LangOptions::SignReturnAddressScopeKind::All
                         ? "all"
                         : "non-leaf");
 
       Fn->addFnAttr("sign-return-address-key",
-                    Key == CodeGenOptions::SignReturnAddressKeyValue::AKey
+                    Key == LangOptions::SignReturnAddressKeyKind::AKey
                         ? "a_key"
                         : "b_key");
     }
 
     if (BranchTargetEnforcement)
       Fn->addFnAttr("branch-target-enforcement");
   }
 };

clang/lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
 
   Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
       Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
 
   Opts.EmitVersionIdentMetadata = Args.hasFlag(OPT_Qy, OPT_Qn, true);
 
   Opts.Addrsig = Args.hasArg(OPT_faddrsig);
 
-  if (Arg *A = Args.getLastArg(OPT_msign_return_address_EQ)) {
-    StringRef SignScope = A->getValue();
-
-    if (SignScope.equals_lower("none"))
-      Opts.setSignReturnAddress(CodeGenOptions::SignReturnAddressScope::None);
-    else if (SignScope.equals_lower("all"))
-      Opts.setSignReturnAddress(CodeGenOptions::SignReturnAddressScope::All);
-    else if (SignScope.equals_lower("non-leaf"))
-      Opts.setSignReturnAddress(
-          CodeGenOptions::SignReturnAddressScope::NonLeaf);
-    else
-      Diags.Report(diag::err_drv_invalid_value)
-          << A->getAsString(Args) << SignScope;
-
-    if (Arg *A = Args.getLastArg(OPT_msign_return_address_key_EQ)) {
-      StringRef SignKey = A->getValue();
-      if (!SignScope.empty() && !SignKey.empty()) {
-        if (SignKey.equals_lower("a_key"))
-          Opts.setSignReturnAddressKey(
-              CodeGenOptions::SignReturnAddressKeyValue::AKey);
-        else if (SignKey.equals_lower("b_key"))
-          Opts.setSignReturnAddressKey(
-              CodeGenOptions::SignReturnAddressKeyValue::BKey);
-        else
-          Diags.Report(diag::err_drv_invalid_value)
-              << A->getAsString(Args) << SignKey;
-      }
-    }
-  }
-
-  Opts.BranchTargetEnforcement = Args.hasArg(OPT_mbranch_target_enforce);
-
   Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
 
   Opts.SpeculativeLoadHardening = Args.hasArg(OPT_mspeculative_load_hardening);
 
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
   Opts.PassPlugins = Args.getAllArgValues(OPT_fpass_plugin_EQ);
 
if (!VerParts.first.startswith("0") &&
           << A->getAsString(Args) << A->getValue();
     }
   }
 
   Opts.CompleteMemberPointers = Args.hasArg(OPT_fcomplete_member_pointers);
   Opts.BuildingPCHWithObjectFile = Args.hasArg(OPT_building_pch_with_obj);
 
   Opts.MaxTokens = getLastArgIntValue(Args, OPT_fmax_tokens_EQ, 0, Diags);
+
+  if (Arg *A = Args.getLastArg(OPT_msign_return_address_EQ)) {
+    StringRef SignScope = A->getValue();
+
+    if (SignScope.equals_lower("none"))
+      Opts.setSignReturnAddressScope(
+          LangOptions::SignReturnAddressScopeKind::None);
+    else if (SignScope.equals_lower("all"))
+      Opts.setSignReturnAddressScope(
+          LangOptions::SignReturnAddressScopeKind::All);
+    else if (SignScope.equals_lower("non-leaf"))
+      Opts.setSignReturnAddressScope(
+          LangOptions::SignReturnAddressScopeKind::NonLeaf);
+    else
+      Diags.Report(diag::err_drv_invalid_value)
+          << A->getAsString(Args) << SignScope;
+
+    if (Arg *A = Args.getLastArg(OPT_msign_return_address_key_EQ)) {
+      StringRef SignKey = A->getValue();
+      if (!SignScope.empty() && !SignKey.empty()) {
+        if (SignKey.equals_lower("a_key"))
+          Opts.setSignReturnAddressKey(
+              LangOptions::SignReturnAddressKeyKind::AKey);
+        else if (SignKey.equals_lower("b_key"))
+          Opts.setSignReturnAddressKey(
+              LangOptions::SignReturnAddressKeyKind::BKey);
+        else
+          Diags.Report(diag::err_drv_invalid_value)
+              << A->getAsString(Args) << SignKey;
+      }
+    }
+  }
+
+  Opts.BranchTargetEnforcement = Args.hasArg(OPT_mbranch_target_enforce);
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
   switch (Action) {
   case frontend::ASTDeclList:
   case frontend::ASTDump:
   case frontend::ASTPrint:
   case frontend::ASTView: