Skip to content

[NFCI][cfi] Refactor into 'SanitizerInfoFromCFICheckKind' #140117

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

Merged
merged 4 commits into from
May 15, 2025

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented May 15, 2025

This refactors existing code into a 'SanitizerInfoFromCFICheckKind' helper function. This will be useful in future work to annotate CFI checks with debug info (#139809).

This refactors existing code into a 'ParseCFITypeCheckKind' helper
function. This will be useful in future work to annotate CFI checks with
debug info (llvm#139809).
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels May 15, 2025
@llvmbot
Copy link
Member

llvmbot commented May 15, 2025

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Thurston Dang (thurstond)

Changes

This refactors existing code into a 'ParseCFITypeCheckKind' helper function. This will be useful in future work to annotate CFI checks with debug info (#139809).


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

2 Files Affected:

  • (modified) clang/lib/CodeGen/CGClass.cpp (+32-24)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+5)
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index befbfc64a680c..3e2725ab06da7 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -2779,6 +2779,37 @@ void CodeGenFunction::EmitTypeMetadataCodeForVCall(const CXXRecordDecl *RD,
   }
 }
 
+std::pair<SanitizerKind::SanitizerOrdinal, llvm::SanitizerStatKind>
+CodeGenFunction::ParseCFITypeCheckKind(CFITypeCheckKind TCK) {
+  SanitizerKind::SanitizerOrdinal M;
+  llvm::SanitizerStatKind SSK;
+
+  switch (TCK) {
+  case CFITCK_VCall:
+    M = SanitizerKind::SO_CFIVCall;
+    SSK = llvm::SanStat_CFI_VCall;
+    break;
+  case CFITCK_NVCall:
+    M = SanitizerKind::SO_CFINVCall;
+    SSK = llvm::SanStat_CFI_NVCall;
+    break;
+  case CFITCK_DerivedCast:
+    M = SanitizerKind::SO_CFIDerivedCast;
+    SSK = llvm::SanStat_CFI_DerivedCast;
+    break;
+  case CFITCK_UnrelatedCast:
+    M = SanitizerKind::SO_CFIUnrelatedCast;
+    SSK = llvm::SanStat_CFI_UnrelatedCast;
+    break;
+  case CFITCK_ICall:
+  case CFITCK_NVMFCall:
+  case CFITCK_VMFCall:
+    llvm_unreachable("unexpected sanitizer kind");
+  }
+
+  return std::make_pair(M, SSK);
+}
+
 void CodeGenFunction::EmitVTablePtrCheckForCall(const CXXRecordDecl *RD,
                                                 llvm::Value *VTable,
                                                 CFITypeCheckKind TCK,
@@ -2842,30 +2873,7 @@ void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD,
       !CGM.HasHiddenLTOVisibility(RD))
     return;
 
-  SanitizerKind::SanitizerOrdinal M;
-  llvm::SanitizerStatKind SSK;
-  switch (TCK) {
-  case CFITCK_VCall:
-    M = SanitizerKind::SO_CFIVCall;
-    SSK = llvm::SanStat_CFI_VCall;
-    break;
-  case CFITCK_NVCall:
-    M = SanitizerKind::SO_CFINVCall;
-    SSK = llvm::SanStat_CFI_NVCall;
-    break;
-  case CFITCK_DerivedCast:
-    M = SanitizerKind::SO_CFIDerivedCast;
-    SSK = llvm::SanStat_CFI_DerivedCast;
-    break;
-  case CFITCK_UnrelatedCast:
-    M = SanitizerKind::SO_CFIUnrelatedCast;
-    SSK = llvm::SanStat_CFI_UnrelatedCast;
-    break;
-  case CFITCK_ICall:
-  case CFITCK_NVMFCall:
-  case CFITCK_VMFCall:
-    llvm_unreachable("unexpected sanitizer kind");
-  }
+  auto [M, SSK] = ParseCFITypeCheckKind(TCK);
 
   std::string TypeName = RD->getQualifiedNameAsString();
   if (getContext().getNoSanitizeList().containsType(
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 7104303cba50e..aac4f0664273e 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3358,6 +3358,11 @@ class CodeGenFunction : public CodeGenTypeCache {
                      SanitizerSet SkippedChecks = SanitizerSet(),
                      llvm::Value *ArraySize = nullptr);
 
+  /// Converts the CFITypeCheckKind into SanitizerKind::SanitizerOrdinal and
+  /// llvm::SanitizerStatKind.
+  static std::pair<SanitizerKind::SanitizerOrdinal, llvm::SanitizerStatKind>
+  ParseCFITypeCheckKind(CFITypeCheckKind TCK);
+
   /// Emit a check that \p Base points into an array object, which
   /// we can access at index \p Index. \p Accessed should be \c false if we
   /// this expression is used as an lvalue, for instance in "&Arr[Idx]".

@thurstond thurstond requested a review from fmayer May 15, 2025 19:41
Copy link
Contributor

@fmayer fmayer left a comment

Choose a reason for hiding this comment

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

lgtm with optional comment

@thurstond thurstond changed the title [NFCI] Refactor into 'ParseCFITypeCheckKind' [NFCI][cfi] Refactor into 'ParseCFITypeCheckKind' May 15, 2025
@thurstond thurstond changed the title [NFCI][cfi] Refactor into 'ParseCFITypeCheckKind' [NFCI][cfi] Refactor into 'SanitizerInfoFromCFICheckKind' May 15, 2025
@thurstond thurstond requested a review from fmayer May 15, 2025 21:23
@thurstond thurstond merged commit b07e19f into llvm:main May 15, 2025
11 checks passed
SanitizerInfoFromCFICheckKind(CodeGenFunction::CFITypeCheckKind TCK) {
switch (TCK) {
case CodeGenFunction::CFITCK_VCall:
return std::make_pair(SanitizerKind::SO_CFIVCall, llvm::SanStat_CFI_VCall);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be
return {SanitizerKind::SO_CFIVCall, llvm::SanStat_CFI_VCall};

@vitalybuka
Copy link
Collaborator

LGTM

thurstond added a commit that referenced this pull request May 19, 2025
…cks (#139809)

This connects the -fsanitize-annotate-debug-info plumbing (#138577) to CFI check codegen, using SanitizerAnnotateDebugInfo() (#139965) and SanitizerInfoFromCFIKind (#140117).

Note: SanitizerAnnotateDebugInfo() is updated to a public function because it is needed in ItaniumCXXABI.

Updates the tests from #139149.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 19, 2025
…for CFI checks (#139809)

This connects the -fsanitize-annotate-debug-info plumbing (llvm/llvm-project#138577) to CFI check codegen, using SanitizerAnnotateDebugInfo() (llvm/llvm-project#139965) and SanitizerInfoFromCFIKind (llvm/llvm-project#140117).

Note: SanitizerAnnotateDebugInfo() is updated to a public function because it is needed in ItaniumCXXABI.

Updates the tests from llvm/llvm-project#139149.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…cks (llvm#139809)

This connects the -fsanitize-annotate-debug-info plumbing (llvm#138577) to CFI check codegen, using SanitizerAnnotateDebugInfo() (llvm#139965) and SanitizerInfoFromCFIKind (llvm#140117).

Note: SanitizerAnnotateDebugInfo() is updated to a public function because it is needed in ItaniumCXXABI.

Updates the tests from llvm#139149.
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
…cks (llvm#139809)

This connects the -fsanitize-annotate-debug-info plumbing (llvm#138577) to CFI check codegen, using SanitizerAnnotateDebugInfo() (llvm#139965) and SanitizerInfoFromCFIKind (llvm#140117).

Note: SanitizerAnnotateDebugInfo() is updated to a public function because it is needed in ItaniumCXXABI.

Updates the tests from llvm#139149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants