[Clang][RISCV] Add preprocessor macros for Zicfilp CFI scheme #109600
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-aarch64 @llvm/pr-subscribers-backend-risc-v Author: Ming-Yi Lai (mylai-mtk) ChangesThis patch adds preprocessor macros when Zicfilp CFI is enabled. The macros are proposed in riscv-non-isa/riscv-c-api-doc#76 , and the CLI flags are from riscv-non-isa/riscv-toolchain-conventions#54. Full diff: https://github.com/llvm/llvm-project/pull/109600.diff 15 Files Affected:
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..feda4260e320c2 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 -fcf-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/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 97573fcf20c1fb..34676e02718fe1 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -817,6 +817,9 @@ def err_drv_experimental_crel : Error<
"-Wa,--allow-experimental-crel must be specified to use -Wa,--crel. "
"CREL is experimental and uses a non-standard section type code">;
+def warn_drv_opt_ignored_by_other : Warning<"ignoring option '%0' due to '%1'">,
+ InGroup<OptionIgnored>;
+
def warn_android_unversioned_fallback : Warning<
"using unversioned Android target directory %0 for target %1; unversioned "
"directories will not be used in Clang 19 -- provide a versioned directory "
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 376d7d4290c0bf..b94f740b45f0cc 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 fcf_branch_label_scheme_EQ : Joined<["-"], "fcf-branch-label-scheme=">,
+ Visibility<[ClangOption, CC1Option]>, Group<f_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.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index a4925e84784af9..9382645d2c7110 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -224,6 +224,26 @@ void RISCVTargetInfo::getTargetDefines(const LangOptions &Opts,
else
Builder.defineMacro("__riscv_32e");
}
+
+ if (ISAInfo->hasExtension("zicfilp") && Opts.CFProtectionBranch) {
+ auto Scheme = Opts.getCFBranchLabelScheme();
+ if (Scheme == CFBranchLabelSchemeKind::Default)
+ Scheme = getDefaultCFBranchLabelScheme();
+
+ switch (Scheme) {
+ case CFBranchLabelSchemeKind::Unlabeled:
+ Builder.defineMacro("__riscv_landing_pad", "1");
+ Builder.defineMacro("__riscv_landing_pad_unlabeled", "1");
+ break;
+ case CFBranchLabelSchemeKind::Default:
+ llvm_unreachable("default cf-branch-label scheme should already be "
+ "transformed to other scheme");
+ case CFBranchLabelSchemeKind::FuncSig:
+ Builder.defineMacro("__riscv_landing_pad", "1");
+ Builder.defineMacro("__riscv_landing_pad_func_sig", "1");
+ break;
+ }
+ }
}
static constexpr Builtin::Info BuiltinInfo[] = {
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index b808ccc8e9cfe9..e375ea78cdf62f 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -131,6 +131,17 @@ class RISCVTargetInfo : public TargetInfo {
bool supportsCpuInit() const override { return getTriple().isOSLinux(); }
bool validateCpuSupports(StringRef Feature) const override;
bool isValidFeatureName(StringRef Name) 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/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index a99ae62984c7d5..77d55257f8c52a 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -273,6 +273,10 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
return TargetInfo::checkCFProtectionBranchSupported(Diags);
};
+ CFBranchLabelSchemeKind getDefaultCFBranchLabelScheme() const override {
+ return CFBranchLabelSchemeKind::Unlabeled;
+ };
+
virtual bool validateOperandSize(const llvm::StringMap<bool> &FeatureMap,
StringRef Constraint, unsigned Size) const;
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 0bab48caf1a5e2..cd16f8d68a5952 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_fcf_branch_label_scheme_EQ))
+ CmdArgs.push_back(Args.MakeArgString(Twine("-fcf-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..397a665d8e7135 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_fcf_branch_label_scheme_EQ, "unlabeled");
+ break;
+ case CFBranchLabelSchemeKind::FuncSig:
+ GenerateArg(Consumer, OPT_fcf_branch_label_scheme_EQ, "func-sig");
+ break;
+ }
+ }
+
if (Opts.FunctionReturnThunks)
GenerateArg(Consumer, OPT_mfunction_return_EQ, "thunk-extern");
@@ -2022,6 +2035,22 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
}
+ if (const Arg *const A = Args.getLastArg(OPT_fcf_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);
+ else
+ Diags.Report(diag::err_drv_invalid_value)
+ << A->getAsString(Args) << Scheme;
+ } else
+ Diags.Report(diag::warn_drv_opt_ignored_by_other)
+ << A->getAsString(Args)
+ << Args.getLastArg(OPT_fcf_protection_EQ)->getAsString(Args);
+ }
+
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 +3981,16 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
}
}
+ if (const Arg *const A = Args.getLastArg(OPT_fcf_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/Preprocessor/riscv-target-features.c b/clang/test/Preprocessor/riscv-target-features.c
index 60675065495bba..ea2d9dd8500f6e 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -1781,3 +1781,35 @@
// RUN: %clang --target=riscv64-unknown-linux-gnu -mcpu=sifive-p450 -E -dM %s \
// RUN: -o - | FileCheck %s --check-prefix=CHECK-MISALIGNED-FAST
// CHECK-MISALIGNED-FAST: __riscv_misaligned_fast 1
+
+// Landing Pad
+
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN: -march=rv32i_zicfilp1p0 -fcf-protection=branch \
+// RUN: -fcf-branch-label-scheme=unlabeled -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-UNLABELED %s
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN: -march=rv64i_zicfilp1p0 -fcf-protection=branch \
+// RUN: -fcf-branch-label-scheme=unlabeled -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-UNLABELED %s
+// CHECK-ZICFILP-UNLABELED-DAG: __riscv_landing_pad 1{{$}}
+// CHECK-ZICFILP-UNLABELED-DAG: __riscv_landing_pad_unlabeled 1{{$}}
+
+// Default cf-branch-label-scheme is func-sig
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN: -march=rv32i_zicfilp1p0 -fcf-protection=branch -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-FUNC-SIG %s
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN: -march=rv64i_zicfilp1p0 -fcf-protection=branch -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-FUNC-SIG %s
+
+// RUN: %clang --target=riscv32 -menable-experimental-extensions \
+// RUN: -march=rv32i_zicfilp1p0 -fcf-protection=branch \
+// RUN: -fcf-branch-label-scheme=func-sig -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-FUNC-SIG %s
+// RUN: %clang --target=riscv64 -menable-experimental-extensions \
+// RUN: -march=rv64i_zicfilp1p0 -fcf-protection=branch \
+// RUN: -fcf-branch-label-scheme=func-sig -E -dM %s -o - \
+// RUN: | FileCheck --check-prefix=CHECK-ZICFILP-FUNC-SIG %s
+// CHECK-ZICFILP-FUNC-SIG-DAG: __riscv_landing_pad 1{{$}}
+// CHECK-ZICFILP-FUNC-SIG-DAG: __riscv_landing_pad_func_sig 1{{$}}
|
kito-cheng
reviewed
Sep 23, 2024
kito-cheng
reviewed
Sep 23, 2024
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
from
0f9f86e to
ce2a105
Compare
|
Update: Address comments |
kito-cheng
reviewed
Sep 23, 2024
topperc
reviewed
Sep 23, 2024
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
from
ce2a105 to
e3ed8f0
Compare
|
Update: Address comments |
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
2 times, most recently
from
bf9ed51 to
9addd94
Compare
topperc
reviewed
Sep 25, 2024
topperc
reviewed
Sep 25, 2024
|
I think maybe we should separate -fcf-protection=branch from the Zicfilp extension being in -march/-mcpu both in the frontend and backend. The encoding for lpad does not require Zicfilp. Only the assembly processing of the "lpad" mnemonic does. Even that we could change. It is useful to protect a binary with an -mcpu that doesn't support Zicfilp. That would make the binary future proof to a CPU that does support it. This is what is done on X86, the CET extension is not a requirement for ENDBR32/ENDBR64 to be inserted, only that the CPU supports multibyte NOPs. |
Generally I would agree that it's a good idea to protect a binary with an @kito-cheng I need your input on this, since this would require a few updates on various PRs to the RISC-V specs. |
Yeah, I just chat with Craig, I think that right way to go, and actually the PR in riscv-toolchain-conventions I has implicitly allow that, I mean I only write |
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
from
9addd94 to
3bc831f
Compare
|
Update: Always support |
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
from
3bc831f to
f87c51a
Compare
These macros allow assembly files to know which CFI label to use when the target has Zicfilp enabled.
mylai-mtk
force-pushed
the
zicfilp-define-macros
branch
from
f87c51a to
35532cf
Compare
|
Hello, is there anything more that I should amend about this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds preprocessor macros when Zicfilp CFI is enabled. The macros are proposed in riscv-non-isa/riscv-c-api-doc#76 , and the CLI flags are from riscv-non-isa/riscv-toolchain-conventions#54.