Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][RISCV] Introduce command line options for RISC-V Zicfilp CFI #109784

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

mylai-mtk
Copy link
Contributor

This patch introduces the CLI flag specified in riscv-non-isa/riscv-toolchain-conventions#54 for RISC-V Zicfilp-backed forward-edge control flow integrity conditioning. The first user of these CLI flags is expected to be #109600 , which adds preprocessor macros when the CFI conditioning is enabled.

(This PR was split from #109600 to separate the commit of flag introduction from macro definition.)

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 24, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang-driver

Author: Ming-Yi Lai (mylai-mtk)

Changes

This patch introduces the CLI flag specified in riscv-non-isa/riscv-toolchain-conventions#54 for RISC-V Zicfilp-backed forward-edge control flow integrity conditioning. The first user of these CLI flags is expected to be #109600 , which adds preprocessor macros when the CFI conditioning is enabled.

(This PR was split from #109600 to separate the commit of flag introduction from macro definition.)


Full diff: https://github.com/llvm/llvm-project/pull/109784.diff

12 Files Affected:

  • (added) clang/include/clang/Basic/CFProtectionOptions.h (+22)
  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+2)
  • (modified) clang/include/clang/Basic/CodeGenOptions.h (+1)
  • (modified) clang/include/clang/Basic/LangOptions.def (+2)
  • (modified) clang/include/clang/Basic/LangOptions.h (+2)
  • (modified) clang/include/clang/Basic/TargetInfo.h (+4)
  • (modified) clang/include/clang/Driver/Options.td (+4)
  • (modified) clang/lib/Basic/TargetInfo.cpp (+5)
  • (modified) clang/lib/Basic/Targets/RISCV.h (+11)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+36)
  • (added) clang/test/CodeGen/RISCV/riscv-cf-protection.c (+58)
diff --git a/clang/include/clang/Basic/CFProtectionOptions.h b/clang/include/clang/Basic/CFProtectionOptions.h
new file mode 100644
index 00000000000000..d97f8489c4631f
--- /dev/null
+++ b/clang/include/clang/Basic/CFProtectionOptions.h
@@ -0,0 +1,22 @@
+//===--- CFProtectionOptions.h ----------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines constants for -fcf-protection and other related flags.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_CFPROTECTIONOPTIONS_H
+#define LLVM_CLANG_BASIC_CFPROTECTIONOPTIONS_H
+
+namespace clang {
+
+enum class CFBranchLabelSchemeKind { Default, Unlabeled, FuncSig };
+
+} // namespace clang
+
+#endif // #ifndef LLVM_CLANG_BASIC_CFPROTECTIONOPTIONS_H
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index b600198998d85b..de7ae73f8a603b 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -110,6 +110,8 @@ CODEGENOPT(CFProtectionReturn , 1, 0) ///< if -fcf-protection is
                                       ///< set to full or return.
 CODEGENOPT(CFProtectionBranch , 1, 0) ///< if -fcf-protection is
                                       ///< set to full or branch.
+ENUM_CODEGENOPT(CFBranchLabelScheme, CFBranchLabelSchemeKind, 2,
+    CFBranchLabelSchemeKind::Default) ///< if -mcf-branch-label-scheme is set.
 CODEGENOPT(FunctionReturnThunks, 1, 0) ///< -mfunction-return={keep|thunk-extern}
 CODEGENOPT(IndirectBranchCSPrefix, 1, 0) ///< if -mindirect-branch-cs-prefix
                                          ///< is set.
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
index f2a707a8ba8d76..a6953c17a447ef 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_CLANG_BASIC_CODEGENOPTIONS_H
 #define LLVM_CLANG_BASIC_CODEGENOPTIONS_H
 
+#include "clang/Basic/CFProtectionOptions.h"
 #include "clang/Basic/PointerAuthOptions.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/XRayInstr.h"
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index fd3346d29f26a3..68db400c22e6c1 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -364,6 +364,8 @@ BENIGN_LANGOPT(CompatibilityQualifiedIdBlockParamTypeChecking, 1, 0,
 LANGOPT(ObjCDisableDirectMethodsForTesting, 1, 0,
         "Disable recognition of objc_direct methods")
 LANGOPT(CFProtectionBranch , 1, 0, "Control-Flow Branch Protection enabled")
+ENUM_LANGOPT(CFBranchLabelScheme, CFBranchLabelSchemeKind, 2, CFBranchLabelSchemeKind::Default,
+             "Control-Flow Branch Protection Label Scheme")
 LANGOPT(FakeAddressSpaceMap , 1, 0, "OpenCL fake address space map")
 ENUM_LANGOPT(AddressSpaceMapMangling , AddrSpaceMapMangling, 2, ASMM_Target, "OpenCL address space map mangling mode")
 LANGOPT(IncludeDefaultHeader, 1, 0, "Include default header file for OpenCL")
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 6c186c410e158d..c3d53ca92d450c 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_BASIC_LANGOPTIONS_H
 #define LLVM_CLANG_BASIC_LANGOPTIONS_H
 
+#include "clang/Basic/CFProtectionOptions.h"
 #include "clang/Basic/CommentOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangStandard.h"
@@ -73,6 +74,7 @@ class LangOptionsBase {
 public:
   using Visibility = clang::Visibility;
   using RoundingMode = llvm::RoundingMode;
+  using CFBranchLabelSchemeKind = clang::CFBranchLabelSchemeKind;
 
   enum GCMode { NonGC, GCOnly, HybridGC };
   enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq };
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index f31d88a354ea28..d96a4fcca56840 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -16,6 +16,7 @@
 
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/BitmaskEnum.h"
+#include "clang/Basic/CFProtectionOptions.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
@@ -1727,6 +1728,9 @@ class TargetInfo : public TransferrableTargetInfo,
   virtual bool
   checkCFProtectionBranchSupported(DiagnosticsEngine &Diags) const;
 
+  /// Get the target default CFBranchLabelScheme scheme
+  virtual CFBranchLabelSchemeKind getDefaultCFBranchLabelScheme() const;
+
   /// Check if the target supports CFProtection return.
   virtual bool
   checkCFProtectionReturnSupported(DiagnosticsEngine &Diags) const;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 002f60350543d9..ccbe302e4937e1 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2841,6 +2841,10 @@ def fcf_protection : Flag<["-"], "fcf-protection">, Group<f_Group>,
   Visibility<[ClangOption, CLOption, CC1Option]>,
   Alias<fcf_protection_EQ>, AliasArgs<["full"]>,
   HelpText<"Enable cf-protection in 'full' mode">;
+def mcf_branch_label_scheme_EQ : Joined<["-"], "mcf-branch-label-scheme=">,
+  Visibility<[ClangOption, CC1Option]>, Group<m_Group>,
+  HelpText<"Select label scheme for branch control-flow architecture protection">,
+  Values<"unlabeled,func-sig">;
 def mfunction_return_EQ : Joined<["-"], "mfunction-return=">,
   Group<m_Group>, Visibility<[ClangOption, CLOption, CC1Option]>,
   HelpText<"Replace returns with jumps to ``__x86_return_thunk`` (x86 only, error otherwise)">,
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index 29f5cd14e46e11..8dfd10984ce685 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -192,6 +192,11 @@ void TargetInfo::resetDataLayout(StringRef DL, const char *ULP) {
   UserLabelPrefix = ULP;
 }
 
+CFBranchLabelSchemeKind TargetInfo::getDefaultCFBranchLabelScheme() const {
+  // if this hook is called, the target should override it
+  llvm::report_fatal_error("not implemented");
+}
+
 bool
 TargetInfo::checkCFProtectionBranchSupported(DiagnosticsEngine &Diags) const {
   Diags.Report(diag::err_opt_not_valid_on_target) << "cf-protection=branch";
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index 351ef21e197c4d..ec0d0bba69eb9f 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -134,6 +134,17 @@ class RISCVTargetInfo : public TargetInfo {
 
   bool validateGlobalRegisterVariable(StringRef RegName, unsigned RegSize,
                                       bool &HasSizeMismatch) const override;
+
+  bool
+  checkCFProtectionBranchSupported(DiagnosticsEngine &Diags) const override {
+    if (ISAInfo->hasExtension("zicfilp"))
+      return true;
+    return TargetInfo::checkCFProtectionBranchSupported(Diags);
+  }
+
+  CFBranchLabelSchemeKind getDefaultCFBranchLabelScheme() const override {
+    return CFBranchLabelSchemeKind::FuncSig;
+  }
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..d3c5c3d9872ee4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7012,6 +7012,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
     CmdArgs.push_back(
         Args.MakeArgString(Twine("-fcf-protection=") + A->getValue()));
+
+    if (Arg *SA = Args.getLastArg(options::OPT_mcf_branch_label_scheme_EQ))
+      CmdArgs.push_back(Args.MakeArgString(Twine("-mcf-branch-label-scheme=") +
+                                           SA->getValue()));
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_mfunction_return_EQ))
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index efd852593468aa..544e7e0203e0a5 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1688,6 +1688,19 @@ void CompilerInvocationBase::GenerateCodeGenArgs(const CodeGenOptions &Opts,
   else if (Opts.CFProtectionBranch)
     GenerateArg(Consumer, OPT_fcf_protection_EQ, "branch");
 
+  if (Opts.CFProtectionBranch) {
+    switch (Opts.getCFBranchLabelScheme()) {
+    case CFBranchLabelSchemeKind::Default:
+      break;
+    case CFBranchLabelSchemeKind::Unlabeled:
+      GenerateArg(Consumer, OPT_mcf_branch_label_scheme_EQ, "unlabeled");
+      break;
+    case CFBranchLabelSchemeKind::FuncSig:
+      GenerateArg(Consumer, OPT_mcf_branch_label_scheme_EQ, "func-sig");
+      break;
+    }
+  }
+
   if (Opts.FunctionReturnThunks)
     GenerateArg(Consumer, OPT_mfunction_return_EQ, "thunk-extern");
 
@@ -2022,6 +2035,19 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
   }
 
+  if (Opts.CFProtectionBranch && T.isRISCV()) {
+    if (const Arg *A = Args.getLastArg(OPT_mcf_branch_label_scheme_EQ)) {
+      const StringRef Scheme = A->getValue();
+      if (Scheme == "unlabeled")
+        Opts.setCFBranchLabelScheme(CFBranchLabelSchemeKind::Unlabeled);
+      else if (Scheme == "func-sig")
+        Opts.setCFBranchLabelScheme(CFBranchLabelSchemeKind::FuncSig);
+      else
+        Diags.Report(diag::err_drv_invalid_value)
+            << A->getAsString(Args) << Scheme;
+    }
+  }
+
   if (const Arg *A = Args.getLastArg(OPT_mfunction_return_EQ)) {
     auto Val = llvm::StringSwitch<llvm::FunctionReturnThunksKind>(A->getValue())
                    .Case("keep", llvm::FunctionReturnThunksKind::Keep)
@@ -3952,6 +3978,16 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
     }
   }
 
+  if (const Arg *A = Args.getLastArg(OPT_mcf_branch_label_scheme_EQ)) {
+    if (Opts.CFProtectionBranch) {
+      const StringRef Scheme = A->getValue();
+      if (Scheme == "unlabeled")
+        Opts.setCFBranchLabelScheme(CFBranchLabelSchemeKind::Unlabeled);
+      else if (Scheme == "func-sig")
+        Opts.setCFBranchLabelScheme(CFBranchLabelSchemeKind::FuncSig);
+    }
+  }
+
   if ((Args.hasArg(OPT_fsycl_is_device) || Args.hasArg(OPT_fsycl_is_host)) &&
       !Args.hasArg(OPT_sycl_std_EQ)) {
     // If the user supplied -fsycl-is-device or -fsycl-is-host, but failed to
diff --git a/clang/test/CodeGen/RISCV/riscv-cf-protection.c b/clang/test/CodeGen/RISCV/riscv-cf-protection.c
new file mode 100644
index 00000000000000..02a22006b1638f
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/riscv-cf-protection.c
@@ -0,0 +1,58 @@
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN:   -march=rv32i_zicfilp1p0 -mcf-branch-label-scheme=unlabeled -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNLABELED-SCHEME-UNUSED %s
+
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN:   -march=rv32i_zicfilp1p0 -mcf-branch-label-scheme=func-sig -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FUNC-SIG-SCHEME-UNUSED %s
+
+// RUN: not %clang --target=riscv32 -fcf-protection=branch \
+// RUN:   -mcf-branch-label-scheme=unlabeled -c %s -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-BRANCH-PROT-INVALID %s
+
+// RUN: not %clang --target=riscv32 -fcf-protection=branch \
+// RUN:   -mcf-branch-label-scheme=func-sig -c %s -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-BRANCH-PROT-INVALID %s
+
+// RUN: %clang --target=riscv32 -mcf-branch-label-scheme=unlabeled -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNLABELED-SCHEME-UNUSED %s
+
+// RUN: %clang --target=riscv32 -mcf-branch-label-scheme=func-sig -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FUNC-SIG-SCHEME-UNUSED %s
+
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN:   -march=rv64i_zicfilp1p0 -mcf-branch-label-scheme=unlabeled -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNLABELED-SCHEME-UNUSED %s
+
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN:   -march=rv64i_zicfilp1p0 -mcf-branch-label-scheme=func-sig -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FUNC-SIG-SCHEME-UNUSED %s
+
+// RUN: not %clang --target=riscv64 -fcf-protection=branch \
+// RUN:   -mcf-branch-label-scheme=unlabeled -c %s -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-BRANCH-PROT-INVALID %s
+
+// RUN: not %clang --target=riscv64 -fcf-protection=branch \
+// RUN:   -mcf-branch-label-scheme=func-sig -c %s -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-BRANCH-PROT-INVALID %s
+
+// RUN: %clang --target=riscv64 -mcf-branch-label-scheme=unlabeled -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-UNLABELED-SCHEME-UNUSED %s
+
+// RUN: %clang --target=riscv64 -mcf-branch-label-scheme=func-sig -c %s \
+// RUN:   -o /dev/null 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FUNC-SIG-SCHEME-UNUSED %s
+
+// CHECK-BRANCH-PROT-INVALID: error: option 'cf-protection=branch' cannot be
+// CHECK-BRANCH-PROT-INVALID-SAME: specified on this target
+// CHECK-UNLABELED-SCHEME-UNUSED: warning: argument unused during compilation:
+// CHECK-UNLABELED-SCHEME-UNUSED-SAME: '-mcf-branch-label-scheme=unlabeled'
+// CHECK-FUNC-SIG-SCHEME-UNUSED: warning: argument unused during compilation:
+// CHECK-FUNC-SIG-SCHEME-UNUSED-SAME: '-mcf-branch-label-scheme=func-sig'

@kito-cheng
Copy link
Member

Could you update this PR? 4579272 is generally LGTM, but I assume this should process within this PR :)

@mylai-mtk
Copy link
Contributor Author

Could you update this PR? 4579272 is generally LGTM, but I assume this should process within this PR :)

Sure, I'm just a few minutes behind you XD

Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…ard-edge CFI

This patch enables the following command line flags for RISC-V targets:

+ `-fcf-protection=branch` turns on forward-edge control-flow integrity conditioning on RISC-V targets with Zicfilp extension
+ `-mcf-branch-label-scheme=unlabeled|func-sig` selects the label scheme used in the forward-edge CFI conditioning
@mylai-mtk mylai-mtk force-pushed the zicfilp-intro-cli-opts branch from 7aaeaef to baa8715 Compare September 26, 2024 09:05
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM

@mylai-mtk
Copy link
Contributor Author

Thanks :D

@mylai-mtk mylai-mtk merged commit 9f33eb8 into llvm:main Sep 26, 2024
8 checks passed
@mylai-mtk mylai-mtk deleted the zicfilp-intro-cli-opts branch October 7, 2024 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:codegen clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants