Skip to content

Conversation

mylai-mtk
Copy link
Contributor

The -fcf-protection flag is now also used to enable CFI features for the RISC-V target, so it's not suitable to define __CET__ solely based on the flag anymore. This patch moves the definition of the __CET__ macro into X86 target hook, so only X86 targets with the -fcf-protection flag would enable the __CET__ macro.

The `-fcf-protection` flag is now also used to enable CFI features for the
RISC-V target, so it's not suitable to define `__CET__` solely based on the
flag anymore. This patch moves the definition of the `__CET__` macro into X86
target hook, so only X86 targets with the `-fcf-protection` flag would enable
the `__CET__` macro.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 18, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 18, 2025

@llvm/pr-subscribers-clang

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

Author: Ming-Yi Lai (mylai-mtk)

Changes

The -fcf-protection flag is now also used to enable CFI features for the RISC-V target, so it's not suitable to define __CET__ solely based on the flag anymore. This patch moves the definition of the __CET__ macro into X86 target hook, so only X86 targets with the -fcf-protection flag would enable the __CET__ macro.


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

3 Files Affected:

  • (modified) clang/lib/Basic/Targets/X86.cpp (+4)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (-11)
  • (modified) clang/test/Preprocessor/riscv-cf-protection-return.c (+2)
diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 84a05cec04e7f..e4d3ad04fe9de 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -1109,6 +1109,10 @@ void X86TargetInfo::getTargetDefines(const LangOptions &Opts,
 
   if (HasFloat128)
     Builder.defineMacro("__SIZEOF_FLOAT128__", "16");
+
+  if (Opts.CFProtectionReturn || Opts.CFProtectionBranch)
+    Builder.defineMacro("__CET__", Twine{(Opts.CFProtectionReturn << 1) |
+                                         Opts.CFProtectionBranch});
 }
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index b9a5c0589ebc4..4eb743acf327f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4776,17 +4776,6 @@ static bool ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
     }
   }
 
-  // Add the __CET__ macro if a CFProtection option is set.
-  if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
-    StringRef Name = A->getValue();
-    if (Name == "branch")
-      Opts.addMacroDef("__CET__=1");
-    else if (Name == "return")
-      Opts.addMacroDef("__CET__=2");
-    else if (Name == "full")
-      Opts.addMacroDef("__CET__=3");
-  }
-
   // Add macros from the command line.
   for (const auto *A : Args.filtered(OPT_D, OPT_U)) {
     if (A->getOption().matches(OPT_D))
diff --git a/clang/test/Preprocessor/riscv-cf-protection-return.c b/clang/test/Preprocessor/riscv-cf-protection-return.c
index 3a93a88fa6839..a4cbaa1edf68c 100644
--- a/clang/test/Preprocessor/riscv-cf-protection-return.c
+++ b/clang/test/Preprocessor/riscv-cf-protection-return.c
@@ -40,5 +40,7 @@
 // RUN: -menable-experimental-extensions -fcf-protection=full -E -dM %s -o - \
 // RUN: | FileCheck --check-prefixes=SHSTK-MACRO %s
 
+// SHSTK-MACRO-NOT: __CET__
 // SHSTK-MACRO: __riscv_shadow_stack 1{{$}}
+// SHSTK-MACRO-NOT: __CET__
 // NO-MACRO-NOT: __riscv_shadow_stack

@mylai-mtk
Copy link
Contributor Author

See #109784 and #112477 for the adoption of -fcf-protection for RISC-V targets

@kito-cheng kito-cheng requested a review from MaskRay February 18, 2025 13:46
@MaskRay
Copy link
Member

MaskRay commented Feb 18, 2025

Can you update the description to mention which PR makes RISCV targets inspect -fcf-protection=?

@mylai-mtk mylai-mtk merged commit f6d74af into llvm:main Feb 19, 2025
12 checks passed
@mylai-mtk mylai-mtk deleted the limit-cet-macro branch February 19, 2025 02:13
@mylai-mtk
Copy link
Contributor Author

Can you update the description to mention which PR makes RISCV targets inspect -fcf-protection=?

Added PR links. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V 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