Skip to content

Commit e06fc2b

Browse files
authored
Fix: Distinguish CFI Metadata Checks in MergeFunctions Pass (llvm#65963)
This diff fixes an issue in the MergeFunctions pass where two different Control Flow Integrity (CFI) metadata checks were incorrectly considered identical. These merges would lead to runtime violations down the line as two separate objects contained a single destructor which itself contained checks for only one of the objects. Here I update the comparison logic to take into account the metadata at llvm.type.test checks. Now, only truly identical checks will be considered for merging, thus preserving the integrity of each check. Previous discussion: https://reviews.llvm.org/D154119
1 parent 658b655 commit e06fc2b

File tree

3 files changed

+105
-1
lines changed

3 files changed

+105
-1
lines changed

llvm/lib/Transforms/IPO/MergeFunctions.cpp

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,9 +375,32 @@ bool MergeFunctions::doFunctionalCheck(std::vector<WeakTrackingVH> &Worklist) {
375375
}
376376
#endif
377377

378+
/// Check whether \p F has an intrinsic which references
379+
/// distinct metadata as an operand. The most common
380+
/// instance of this would be CFI checks for function-local types.
381+
static bool hasDistinctMetadataIntrinsic(const Function &F) {
382+
for (const BasicBlock &BB : F) {
383+
for (const Instruction &I : BB.instructionsWithoutDebug()) {
384+
if (!isa<IntrinsicInst>(&I))
385+
continue;
386+
387+
for (Value *Op : I.operands()) {
388+
auto *MDL = dyn_cast<MetadataAsValue>(Op);
389+
if (!MDL)
390+
continue;
391+
if (MDNode *N = dyn_cast<MDNode>(MDL->getMetadata()))
392+
if (N->isDistinct())
393+
return true;
394+
}
395+
}
396+
}
397+
return false;
398+
}
399+
378400
/// Check whether \p F is eligible for function merging.
379401
static bool isEligibleForMerging(Function &F) {
380-
return !F.isDeclaration() && !F.hasAvailableExternallyLinkage();
402+
return !F.isDeclaration() && !F.hasAvailableExternallyLinkage() &&
403+
!hasDistinctMetadataIntrinsic(F);
381404
}
382405

383406
bool MergeFunctions::runOnModule(Module &M) {
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
2+
;; Check the cases involving internal CFI instrumented functions where we do not expect functions to be merged.
3+
; RUN: opt -S -passes=mergefunc < %s | FileCheck %s
4+
5+
; Function Attrs: nocallback nofree nosync nounwind readnone speculatable willreturn
6+
declare i1 @llvm.type.test(ptr, metadata) #6
7+
8+
define internal void @A__on_zero_sharedEv(ptr noundef nonnull align 8 dereferenceable(32) %this) {
9+
; CHECK-LABEL: define internal void @A__on_zero_sharedEv
10+
; CHECK-SAME: (ptr noundef nonnull align 8 dereferenceable(32) [[THIS:%.*]]) {
11+
; CHECK-NEXT: entry:
12+
; CHECK-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8
13+
; CHECK-NEXT: store ptr [[THIS]], ptr [[THIS_ADDR]], align 8
14+
; CHECK-NEXT: [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8
15+
; CHECK-NEXT: [[VTABLE:%.*]] = load ptr, ptr [[THIS1]], align 8
16+
; CHECK-NEXT: [[TMP0:%.*]] = call i1 @llvm.type.test(ptr [[VTABLE]], metadata [[META0:![0-9]+]]), !nosanitize !1
17+
; CHECK-NEXT: ret void
18+
;
19+
entry:
20+
%this.addr = alloca ptr, align 8
21+
store ptr %this, ptr %this.addr, align 8
22+
%this1 = load ptr, ptr %this.addr, align 8
23+
%vtable = load ptr, ptr %this1, align 8
24+
%0 = call i1 @llvm.type.test(ptr %vtable, metadata !11), !nosanitize !47
25+
ret void
26+
}
27+
28+
; Function Attrs: mustprogress noinline nounwind optnone uwtable
29+
define internal void @B__on_zero_sharedEv(ptr noundef nonnull align 8 dereferenceable(32) %this) {
30+
; CHECK-LABEL: define internal void @B__on_zero_sharedEv
31+
; CHECK-SAME: (ptr noundef nonnull align 8 dereferenceable(32) [[THIS:%.*]]) {
32+
; CHECK-NEXT: entry:
33+
; CHECK-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8
34+
; CHECK-NEXT: store ptr [[THIS]], ptr [[THIS_ADDR]], align 8
35+
; CHECK-NEXT: [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8
36+
; CHECK-NEXT: [[VTABLE:%.*]] = load ptr, ptr [[THIS1]], align 8
37+
; CHECK-NEXT: [[TMP0:%.*]] = call i1 @llvm.type.test(ptr [[VTABLE]], metadata [[META2:![0-9]+]]), !nosanitize !1
38+
; CHECK-NEXT: ret void
39+
;
40+
entry:
41+
%this.addr = alloca ptr, align 8
42+
store ptr %this, ptr %this.addr, align 8
43+
%this1 = load ptr, ptr %this.addr, align 8
44+
%vtable = load ptr, ptr %this1, align 8
45+
%0 = call i1 @llvm.type.test(ptr %vtable, metadata !22), !nosanitize !47
46+
ret void
47+
}
48+
49+
!10 = !{i64 16, !11}
50+
!11 = distinct !{}
51+
!21 = !{i64 16, !22}
52+
!22 = distinct !{}
53+
!47 = !{}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
2+
;; Make sure internal constrained FP intrinsics still merge properly
3+
; RUN: opt -passes=mergefunc -S < %s | FileCheck %s
4+
5+
declare float @llvm.experimental.constrained.fadd.f32(float, float, metadata, metadata)
6+
7+
define float @func1(float %a, float %b) {
8+
; CHECK-LABEL: define float @func1
9+
; CHECK-SAME: (float [[A:%.*]], float [[B:%.*]]) {
10+
; CHECK-NEXT: [[RESULT:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float [[A]], float [[B]], metadata !"round.dynamic", metadata !"fpexcept.strict")
11+
; CHECK-NEXT: [[RESULT_2:%.*]] = call float @llvm.experimental.constrained.fadd.f32(float [[A]], float [[B]], metadata !"round.dynamic", metadata !"fpexcept.strict")
12+
; CHECK-NEXT: ret float [[RESULT]]
13+
;
14+
%result = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
15+
%result_2 = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
16+
ret float %result
17+
}
18+
19+
define float @func2(float %a, float %b) {
20+
; CHECK-LABEL: define float @func2
21+
; CHECK-SAME: (float [[TMP0:%.*]], float [[TMP1:%.*]]) {
22+
; CHECK-NEXT: [[TMP3:%.*]] = tail call float @func1(float [[TMP0]], float [[TMP1]])
23+
; CHECK-NEXT: ret float [[TMP3]]
24+
;
25+
%result = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
26+
%result_2 = call float @llvm.experimental.constrained.fadd.f32(float %a, float %b, metadata !"round.dynamic", metadata !"fpexcept.strict")
27+
ret float %result
28+
}

0 commit comments

Comments
 (0)