Skip to content

[X86][KCFI] Do not emit a type prefix for nocf_check functions #158133

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[X86][KCFI] Do not emit a type prefix for nocf_check functions #158133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4a52f4
[X86][KCFI] Do not emit a type prefix for nocf_check functions
samitolvanen Sep 11, 2025
55c9356
Also allow nocf_check only with -fsanitize=kcfi
samitolvanen Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2028,7 +2028,8 @@ static void handleStandardNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &A) {
}

static void handleNoCfCheckAttr(Sema &S, Decl *D, const ParsedAttr &Attrs) {
if (!S.getLangOpts().CFProtectionBranch)
if (!S.getLangOpts().CFProtectionBranch &&
!S.getLangOpts().Sanitize.has(SanitizerKind::KCFI))
S.Diag(Attrs.getLoc(), diag::warn_nocf_check_attribute_ignored);
else
handleSimpleAttribute<AnyX86NoCfCheckAttr>(S, D, Attrs);
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/SemaType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7936,7 +7936,8 @@ static bool handleFunctionTypeAttr(TypeProcessingState &state, ParsedAttr &attr,
}

if (attr.getKind() == ParsedAttr::AT_AnyX86NoCfCheck) {
if (!S.getLangOpts().CFProtectionBranch) {
if (!S.getLangOpts().CFProtectionBranch &&
!S.getLangOpts().Sanitize.has(SanitizerKind::KCFI)) {
S.Diag(attr.getLoc(), diag::warn_nocf_check_attribute_ignored);
attr.setInvalid();
return true;
Expand Down
1 change: 1 addition & 0 deletions clang/test/Sema/attr-nocf_check.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -verify -fcf-protection=branch -fsyntax-only %s
// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -verify -fsanitize=kcfi -fsyntax-only %s

// Function pointer definition.
typedef void (*FuncPointerWithNoCfCheck)(void) __attribute__((nocf_check)); // no-warning
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/X86/X86AsmPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,9 @@ void X86AsmPrinter::emitKCFITypeId(const MachineFunction &MF) {
Type = mdconst::extract<ConstantInt>(MD->getOperand(0));

// If we don't have a type to emit, just emit padding if needed to maintain
// the same alignment for all functions.
if (!Type) {
// the same alignment for all functions. Also skip `nocf_check` functions as
// they are not indirectly callable due to a missing ENDBR.
if (!Type || F.doesNoCfCheck()) {
EmitKCFITypePadding(MF, /*HasType=*/false);
return;
}
Expand Down
39 changes: 39 additions & 0 deletions llvm/test/CodeGen/X86/kcfi-nocf-check.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
; RUN: llc -mtriple=x86_64-unknown-unknown -x86-indirect-branch-tracking < %s | FileCheck %s

; CHECK-LABEL: __cfi_cf_check_func:
; CHECK: movl $12345678, %eax
define void @cf_check_func() !kcfi_type !2 {
; CHECK-LABEL: cf_check_func:
; CHECK: endbr64
; CHECK: retq
entry:
ret void
}

; CHECK-NOT: __cfi_notype_cf_check_func:
Copy link
Member

Choose a reason for hiding this comment

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

We just need one CHECK-NOT: __cfi_ and the next ; CHECK-NOT: __cfi_nocf_check_func: can be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

We just need one CHECK-NOT: __cfi_ and the next ; CHECK-NOT: __cfi_nocf_check_func: can be removed.

Do you mean something like this?

; CHECK-NOT:   __cfi_
define void @notype_cf_check_func() {
; CHECK-LABEL: notype_cf_check_func:
; CHECK:       endbr64
; CHECK:       retq
entry:
  ret void
}

define void @nocf_check_func() #0 !kcfi_type !2 {
; CHECK-LABEL: nocf_check_func:
; CHECK-NOT:   endbr64
; CHECK:       retq
entry:
  ret void
}

Perhaps I misunderstood your comment, because if we now end up emitting a __cfi_ type prefix before nocf_check_func, FileCheck is not going to detect that. We're still going to need a second CHECK-NOT: __cfi_ before nocf_check_func to catch this case.

Am I missing some clever FileCheck trick that would make this simpler?

; CHECK-NOT: movl
define void @notype_cf_check_func() {
; CHECK-LABEL: notype_cf_check_func:
; CHECK: endbr64
; CHECK: retq
entry:
ret void
}

; CHECK-NOT: __cfi_nocf_check_func:
; CHECK-NOT: movl
define void @nocf_check_func() #0 !kcfi_type !2 {
; CHECK-LABEL: nocf_check_func:
; CHECK-NOT: endbr64
; CHECK: retq
entry:
ret void
}

attributes #0 = { nocf_check }

!llvm.module.flags = !{!0, !1}

!0 = !{i32 8, !"cf-protection-branch", i32 1}
!1 = !{i32 4, !"kcfi", i32 1}
!2 = !{i32 12345678}