[X86][test] Add tests for -fpatchable-function-entry=N,M (where M>0) and its interaction with -fcf-protection=branch

[MaskRay]

Summary

And add tests for its interaction with -fcf-protection=branch

[merge_guards_bot]

Unit tests: fail. 62040 tests passed, 2 failed and 783 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_class/try_lock.pass.cpp
failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/lock.pass.cpp

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[nickdesaulniers]

llvm/lib/Target/X86/X86MCInstLower.cpp

2646 ↗

(On Diff #239212)

See comments on D73070 about making nop emission generic.

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

39

Need to know where .Lfunc_end2 is placed, please add a CHECK for it.

llvm/test/CodeGen/X86/patchable-function-entry.ll

89

Ditto.

[MaskRay]

Delete code change.
This is a pure test patch now.

[merge_guards_bot]

Unit tests: fail. 62081 tests passed, 1 failed and 784 were skipped.

failed: Clang.Driver/cc-print-options.c

clang-tidy: unknown.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[nickdesaulniers]

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

28

Should the filename of this added test be s/ibt.ll/bti.ll?

llvm/test/CodeGen/X86/patchable-function-entry.ll

95

The comment about -ffunction-sections added to the aarch64 test in https://reviews.llvm.org/D73070 would be helpful here, too.

[MaskRay]

Improve tests

[MaskRay]

Fix a typo

-mbranch-protection=bti -> -fcf-protection=branch

[MaskRay]

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

28

Sorry, I made a typo.

It should be -fcf-protection=branch (https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html)

This is the Intel technology which resembles to AArch64's IBT.

[nickdesaulniers]

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

28

Yes, and I think the name of the test is still misspelled, I think.

Consider the test modified in https://reviews.llvm.org/D73070 named llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll, but this file is named llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll. bti vs ibt. Not only is the order different, but bti is only (?) aarch64 specific, so maybe the filename of this test should have fcf, not bti (or ibt).

[merge_guards_bot]

Unit tests: pass. 62082 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[merge_guards_bot]

Unit tests: pass. 62083 tests passed, 0 failed and 784 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[MaskRay]

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

28

AArch64: Branch Target Identification (bti)
x86: Indirect Branch Tracking (ibt) (I recently added an lld option -z force-ibt, so I am confident here...)

I think the filename is correct, but my comment above made a typo...

This is the Intel technology which resembles to AArch64's IBT.

IBT->BTI.

[nickdesaulniers]

Ah, sorry then for my misunderstanding.

llvm/test/CodeGen/X86/patchable-function-entry-ibt.ll

+; RUN: llc -mtriple=x86_64 %s -o - | FileCheck --check-prefixes=CHECK %s
+
+;; -fpatchable-function-entry=0 -fcf-protection=branch
+define void @f0() "patchable-function-entry"="0" "branch-target-enforcement" {
+; CHECK-LABEL: f0:
+; CHECK-NEXT: .Lfunc_begin0:
+; CHECK:      # %bb.0:
+; CHECK-NEXT:  endbr64
+; CHECK-NEXT:  retq
+; CHECK-NOT:  .section __patchable_function_entries
+  ret void
+}
+
+;; -fpatchable-function-entry=1 -fcf-protection=branch
+define void @f1() "patchable-function-entry"="1" {
+; CHECK-LABEL: f1:
+; CHECK-NEXT: .Lfunc_begin1:
+; CHECK:       endbr64
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  retq
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK-NEXT: .p2align 3
+; CHECK-NEXT: .quad .Lfunc_begin1
+  ret void
+}
+
+;; -fpatchable-function-entry=2,1 -fcf-protection=branch
+define void @f2_1() "patchable-function-entry"="1" "patchable-function-prefix"="1" {
+; CHECK-LABEL: .type f2_1,@function
+; CHECK-NEXT: .Ltmp0:
+; CHECK-NEXT:  nop
+; CHECK-NEXT: f2_1:
+; CHECK-NEXT: .Lfunc_begin2:
+; CHECK:      # %bb.0:
+; CHECK-NEXT:  endbr64
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  retq
+; CHECK:      .Lfunc_end2:
+; CHECK-NEXT: .size f2_1, .Lfunc_end2-f2_1
+; CHECK:      .section __patchable_function_entries,"awo",@progbits,f1,unique,0
+; CHECK-NEXT: .p2align 3
+; CHECK-NEXT: .quad .Ltmp0
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"cf-protection-branch", i32 1}

llvm/test/CodeGen/X86/patchable-function-entry.ll

 ; CHECK:       .section __patchable_function_entries,"awo",@progbits,f1,unique,0
 ; 32:          .p2align 2
 ; 32-NEXT:     .long .Lfunc_begin1
 ; 64:          .p2align 3
 ; 64-NEXT:     .quad .Lfunc_begin1
   ret void
 }
 
+;; Without -function-sections, f2 is in the same text section as f1.
+;; They share the __patchable_function_entries section.
+;; With -function-sections, f1 and f2 are in different text sections.
+;; Use separate __patchable_function_entries.
 define void @f2() "patchable-function-entry"="2" {
 ; CHECK-LABEL: f2:
+; CHECK-NEXT: .Lfunc_begin2:
 ; 32-COUNT-2:  nop
 ; 64:          xchgw %ax, %ax
 ; CHECK-NEXT:  ret
 ; NOFSECT:     .section __patchable_function_entries,"awo",@progbits,f1,unique,0
 ; FSECT:       .section __patchable_function_entries,"awo",@progbits,f2,unique,1
 ; 32:          .p2align 2
 ; 32-NEXT:     .long .Lfunc_begin2
 ; 64:          .p2align 3
 ; 64-NEXT:     .quad .Lfunc_begin2
   ret void
 }
 
 $f3 = comdat any
 define void @f3() "patchable-function-entry"="3" comdat {
 ; CHECK-LABEL: f3:
+; CHECK-NEXT: .Lfunc_begin3:
 ; 32-COUNT-3:  nop
 ; 64:          nopl (%rax)
 ; CHECK:       ret
 ; NOFSECT:     .section __patchable_function_entries,"aGwo",@progbits,f3,comdat,f3,unique,1
 ; FSECT:       .section __patchable_function_entries,"aGwo",@progbits,f3,comdat,f3,unique,2
 ; 32:          .p2align 2
 ; 32-NEXT:     .long .Lfunc_begin3
 ; 64:          .p2align 3
 ; 64-NEXT:     .quad .Lfunc_begin3
   ret void
 }
 
 $f5 = comdat any
 define void @f5() "patchable-function-entry"="5" comdat {
 ; CHECK-LABEL: f5:
+; CHECK-NEXT: .Lfunc_begin4:
 ; 32-COUNT-5:  nop
 ; 64:          nopl 8(%rax,%rax)
 ; CHECK-NEXT:  ret
 ; NOFSECT      .section __patchable_function_entries,"aGwo",@progbits,f5,comdat,f5,unique,2
 ; FSECT:       .section __patchable_function_entries,"aGwo",@progbits,f5,comdat,f5,unique,3
 ; 32:          .p2align 2
 ; 32-NEXT:     .long .Lfunc_begin4
 ; 64:          .p2align 3
 ; 64-NEXT:     .quad .Lfunc_begin4
   ret void
 }
+
+;; -fpatchable-function-entry=3,2
+;; "patchable-function-prefix" emits data before the function entry label.
+;; We emit 1-byte NOPs before the function entry, so that with a partial patch,
+;; the remaining instructions do not need to be modified.
+define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"="2" {
+; CHECK-LABEL: .type f3_2,@function
+; CHECK-NEXT: .Ltmp0: # @f3_2
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  nop
+; CHECK-NEXT: f3_2:
+; CHECK:      # %bb.0:
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  ret
+;; .size does not include the prefix.
+; CHECK:      .Lfunc_end5:
+; CHECK-NEXT: .size f3_2, .Lfunc_end5-f3_2
+; NOFSECT     .section __patchable_function_entries,"awo",@progbits,f0,unique,0
+; FSECT:      .section __patchable_function_entries,"awo",@progbits,f3_2,unique,4
+; 32:         .p2align 2
+; 32-NEXT:    .long .Ltmp0
+; 64:         .p2align 3
+; 64-NEXT:    .quad .Ltmp0
+  %frame = alloca i8, i32 16
+  ret void
+}