Skip to content

Commit 6767029

Browse files
pccpcc
committed
Improve coerce logic and add coerce test
Created using spr 1.3.6-beta.1
2 parents 95b5e4e + c1d4e85 commit 6767029

File tree

8 files changed

+167
-34
lines changed

8 files changed

+167
-34
lines changed

‎clang/include/clang/AST/ASTContext.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ struct TypeInfoChars {
186186
struct PFPField {
187187
CharUnits offset;
188188
FieldDecl *field;
189+
bool isWithinUnion;
189190
};
190191

191192
/// Holds long-lived AST nodes (such as types and decls) that can be
@@ -3727,7 +3728,8 @@ OPT_LIST(V)
37273728

37283729
bool isPFPStruct(const RecordDecl *rec) const;
37293730
void findPFPFields(QualType Ty, CharUnits Offset,
3730-
std::vector<PFPField> &Fields, bool IncludeVBases) const;
3731+
std::vector<PFPField> &Fields, bool IncludeVBases,
3732+
bool IsWithinUnion = false) const;
37313733
bool hasPFPFields(QualType ty) const;
37323734
bool isPFPField(const FieldDecl *field) const;
37333735

‎clang/lib/AST/ASTContext.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15183,6 +15183,10 @@ bool ASTContext::useAbbreviatedThunkName(GlobalDecl VirtualMethodDecl,
1518315183
}
1518415184

1518515185
bool ASTContext::arePFPFieldsTriviallyRelocatable(const RecordDecl *RD) const {
15186+
bool IsPAuthSupported =
15187+
getTargetInfo().getTriple().getArch() == llvm::Triple::aarch64;
15188+
if (!IsPAuthSupported)
15189+
return true;
1518615190
if (getLangOpts().getPointerFieldProtection() ==
1518715191
LangOptions::PointerFieldProtectionKind::Tagged)
1518815192
return !isa<CXXRecordDecl>(RD) ||
@@ -15200,7 +15204,7 @@ bool ASTContext::isPFPStruct(const RecordDecl *rec) const {
1520015204

1520115205
void ASTContext::findPFPFields(QualType Ty, CharUnits Offset,
1520215206
std::vector<PFPField> &Fields,
15203-
bool IncludeVBases) const {
15207+
bool IncludeVBases, bool IsWithinUnion) const {
1520415208
if (auto *AT = getAsConstantArrayType(Ty)) {
1520515209
if (auto *ElemDecl = AT->getElementType()->getAsCXXRecordDecl()) {
1520615210
const ASTRecordLayout &ElemRL = getASTRecordLayout(ElemDecl);
@@ -15213,26 +15217,27 @@ void ASTContext::findPFPFields(QualType Ty, CharUnits Offset,
1521315217
auto *Decl = Ty->getAsCXXRecordDecl();
1521415218
if (!Decl)
1521515219
return;
15220+
IsWithinUnion |= Decl->isUnion();
1521615221
const ASTRecordLayout &RL = getASTRecordLayout(Decl);
1521715222
for (FieldDecl *field : Decl->fields()) {
1521815223
CharUnits fieldOffset =
1521915224
Offset + toCharUnitsFromBits(RL.getFieldOffset(field->getFieldIndex()));
1522015225
if (isPFPField(field))
15221-
Fields.push_back({fieldOffset, field});
15222-
findPFPFields(field->getType(), fieldOffset, Fields, true);
15226+
Fields.push_back({fieldOffset, field, IsWithinUnion});
15227+
findPFPFields(field->getType(), fieldOffset, Fields, true, IsWithinUnion);
1522315228
}
1522415229
for (auto &Base : Decl->bases()) {
1522515230
if (Base.isVirtual())
1522615231
continue;
1522715232
CharUnits BaseOffset =
1522815233
Offset + RL.getBaseClassOffset(Base.getType()->getAsCXXRecordDecl());
15229-
findPFPFields(Base.getType(), BaseOffset, Fields, false);
15234+
findPFPFields(Base.getType(), BaseOffset, Fields, false, IsWithinUnion);
1523015235
}
1523115236
if (IncludeVBases) {
1523215237
for (auto &Base : Decl->vbases()) {
1523315238
CharUnits BaseOffset =
1523415239
Offset + RL.getVBaseClassOffset(Base.getType()->getAsCXXRecordDecl());
15235-
findPFPFields(Base.getType(), BaseOffset, Fields, false);
15240+
findPFPFields(Base.getType(), BaseOffset, Fields, false, IsWithinUnion);
1523615241
}
1523715242
}
1523815243
}

‎clang/lib/CodeGen/CGBuiltin.cpp

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4499,14 +4499,13 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
44994499
Address Dest = EmitPointerWithAlignment(E->getArg(0));
45004500
Address Src = EmitPointerWithAlignment(E->getArg(1));
45014501
Value *SizeVal = EmitScalarExpr(E->getArg(2));
4502+
Value *TypeSize = ConstantInt::get(
4503+
SizeVal->getType(),
4504+
getContext()
4505+
.getTypeSizeInChars(E->getArg(0)->getType()->getPointeeType())
4506+
.getQuantity());
45024507
if (BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_trivially_relocate)
4503-
SizeVal = Builder.CreateMul(
4504-
SizeVal,
4505-
ConstantInt::get(
4506-
SizeVal->getType(),
4507-
getContext()
4508-
.getTypeSizeInChars(E->getArg(0)->getType()->getPointeeType())
4509-
.getQuantity()));
4508+
SizeVal = Builder.CreateMul(SizeVal, TypeSize);
45104509
EmitArgCheck(TCK_Store, Dest, E->getArg(0), 0);
45114510
EmitArgCheck(TCK_Load, Src, E->getArg(1), 1);
45124511
auto *I = Builder.CreateMemMove(Dest, Src, SizeVal, false);
@@ -4515,13 +4514,38 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
45154514
std::vector<PFPField> PFPFields;
45164515
getContext().findPFPFields(E->getArg(0)->getType()->getPointeeType(),
45174516
CharUnits::Zero(), PFPFields, true);
4518-
for (auto &Field : PFPFields) {
4519-
if (getContext().arePFPFieldsTriviallyRelocatable(
4520-
Field.field->getParent()))
4521-
continue;
4522-
auto DestFieldPtr = EmitAddressOfPFPField(Dest, Field);
4523-
auto SrcFieldPtr = EmitAddressOfPFPField(Src, Field);
4524-
Builder.CreateStore(Builder.CreateLoad(SrcFieldPtr), DestFieldPtr);
4517+
if (!PFPFields.empty()) {
4518+
BasicBlock *Entry = Builder.GetInsertBlock();
4519+
BasicBlock *Loop = createBasicBlock("loop");
4520+
BasicBlock *LoopEnd = createBasicBlock("loop.end");
4521+
Builder.CreateCondBr(
4522+
Builder.CreateICmpEQ(SizeVal,
4523+
ConstantInt::get(SizeVal->getType(), 0)),
4524+
LoopEnd, Loop);
4525+
4526+
EmitBlock(Loop);
4527+
PHINode *Offset = Builder.CreatePHI(SizeVal->getType(), 2);
4528+
Offset->addIncoming(ConstantInt::get(SizeVal->getType(), 0), Entry);
4529+
Address DestRec = Dest.withPointer(
4530+
Builder.CreateInBoundsGEP(Int8Ty, Dest.getBasePointer(), {Offset}),
4531+
KnownNonNull);
4532+
Address SrcRec = Src.withPointer(
4533+
Builder.CreateInBoundsGEP(Int8Ty, Src.getBasePointer(), {Offset}),
4534+
KnownNonNull);
4535+
for (auto &Field : PFPFields) {
4536+
if (getContext().arePFPFieldsTriviallyRelocatable(
4537+
Field.field->getParent()))
4538+
continue;
4539+
auto DestFieldPtr = EmitAddressOfPFPField(DestRec, Field);
4540+
auto SrcFieldPtr = EmitAddressOfPFPField(SrcRec, Field);
4541+
Builder.CreateStore(Builder.CreateLoad(SrcFieldPtr), DestFieldPtr);
4542+
}
4543+
4544+
Value *NextOffset = Builder.CreateAdd(Offset, TypeSize);
4545+
Offset->addIncoming(NextOffset, Loop);
4546+
Builder.CreateCondBr(Builder.CreateICmpEQ(NextOffset, SizeVal), LoopEnd, Loop);
4547+
4548+
EmitBlock(LoopEnd);
45254549
}
45264550
}
45274551
return RValue::get(Dest, *this);

‎clang/lib/CodeGen/CGExpr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5112,6 +5112,7 @@ static Address emitRawAddrOfFieldStorage(CodeGenFunction &CGF, Address base,
51125112
return emitAddrOfZeroSizeField(CGF, base, field, IsInBounds);
51135113

51145114
const RecordDecl *rec = field->getParent();
5115+
51155116
unsigned idx =
51165117
CGF.CGM.getTypes().getCGRecordLayout(rec).getLLVMFieldNo(field);
51175118

‎clang/lib/Sema/SemaTypeTraits.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -235,16 +235,16 @@ static bool IsEligibleForReplacement(Sema &SemaRef, const CXXRecordDecl *D) {
235235

236236
static bool IsImplementationDefinedNonRelocatable(Sema &SemaRef,
237237
const CXXRecordDecl *D) {
238-
// FIXME: Should also check for polymorphic union members here if PAuth ABI is
239-
// enabled.
240-
241-
// FIXME: PFP should not affect trivial relocatability except in cases where a
242-
// PFP field is a member of a union, instead it should affect the
243-
// implementation of std::trivially_relocate. See:
244-
// https://discourse.llvm.org/t/rfc-structure-protection-a-family-of-uaf-mitigation-techniques/85555/16?u=pcc
245-
if (!SemaRef.Context.arePFPFieldsTriviallyRelocatable(D) &&
246-
SemaRef.Context.hasPFPFields(QualType(D->getTypeForDecl(), 0)))
247-
return true;
238+
// The implementation-defined carveout only exists for polymorphic types.
239+
if (!D->isPolymorphic())
240+
return false;
241+
242+
std::vector<PFPField> pfpFields;
243+
SemaRef.Context.findPFPFields(QualType(D->getTypeForDecl(), 0),
244+
CharUnits::Zero(), pfpFields, true);
245+
for (PFPField f : pfpFields)
246+
if (f.isWithinUnion)
247+
return true;
248248

249249
return false;
250250
}

‎clang/test/CodeGenCXX/pfp-coerce.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ void pass_trivial_abi_pointer(TrivialAbiPointer p, TrivialAbiPointer *pp) {
186186
// X86_64: store ptr %p.coerce, ptr %1, align 8
187187
// X86_64: store ptr %pp, ptr %pp.addr, align 8
188188
// X86_64: %2 = load ptr, ptr %pp.addr, align 8
189-
// X86_64: %call = call noundef nonnull align 8 dereferenceable(8) ptr @_ZN17TrivialAbiPointeraSERKS_(ptr noundef nonnull align 8 dereferenceable(8) %2, ptr noundef nonnull align 8 dereferenceable(8) %p)
189+
// X86_64: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %2, ptr align 8 %p, i64 8, i1 false)
190190
// X86_64: call void @_ZN17TrivialAbiPointerD1Ev(ptr noundef nonnull align 8 dereferenceable(8) %p)
191191
*pp = p;
192192
}
@@ -210,7 +210,7 @@ TrivialAbiPointer return_trivial_abi_pointer(TrivialAbiPointer *pp) {
210210
// X86_64: %pp.addr = alloca ptr, align 8
211211
// X86_64: store ptr %pp, ptr %pp.addr, align 8
212212
// X86_64: %0 = load ptr, ptr %pp.addr, align 8
213-
// X86_64: call void @_ZN17TrivialAbiPointerC1ERKS_(ptr noundef nonnull align 8 dereferenceable(8) %retval, ptr noundef nonnull align 8 dereferenceable(8) %0)
213+
// X86_64: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %retval, ptr align 8 %0, i64 8, i1 false)
214214
// X86_64: %1 = getelementptr inbounds i8, ptr %retval, i64 0
215215
// X86_64: %2 = call ptr @llvm.protected.field.ptr(ptr %1, i64 33, i1 false) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS17TrivialAbiPointer.ptr) ]
216216
// X86_64: %3 = load ptr, ptr %2, align 8

‎clang/test/CodeGenCXX/pfp-memcpy.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -fexperimental-pointer-field-protection=tagged -emit-llvm -o - %s | FileCheck %s
1+
// RUN: %clang_cc1 -triple aarch64-linux -fexperimental-pointer-field-protection=tagged -emit-llvm -o - %s | FileCheck %s
22

33
struct ClassWithTrivialCopy {
44
ClassWithTrivialCopy();
@@ -16,4 +16,4 @@ void make_trivial_copy(ClassWithTrivialCopy *s1, ClassWithTrivialCopy *s2) {
1616

1717
// CHECK-LABEL: define{{.*}} void @_Z17make_trivial_copyP20ClassWithTrivialCopyS0_
1818
// CHECK-NOT: memcpy
19-
// CHECK: ret void
19+
// CHECK: ret void

‎clang/test/CodeGenCXX/pfp-trivially-relocatable.cpp

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// RUN: %clang_cc1 -std=c++26 -triple x86_64-linux-gnu -emit-llvm -fexperimental-pointer-field-protection=untagged -o - %s | FileCheck --check-prefix=RELOC %s
2+
// RUN: %clang_cc1 -std=c++26 -triple x86_64-linux-gnu -emit-llvm -fexperimental-pointer-field-protection=tagged -o - %s | FileCheck --check-prefix=RELOC %s
3+
// RUN: %clang_cc1 -std=c++26 -triple aarch64-linux-gnu -emit-llvm -fexperimental-pointer-field-protection=untagged -o - %s | FileCheck --check-prefix=RELOC %s
4+
// RUN: %clang_cc1 -std=c++26 -triple aarch64-linux-gnu -emit-llvm -fexperimental-pointer-field-protection=tagged -o - %s | FileCheck --check-prefix=NONRELOC %s
5+
6+
typedef __SIZE_TYPE__ size_t;
7+
8+
struct S trivially_relocatable_if_eligible {
9+
S(const S&);
10+
~S();
11+
int* a;
12+
private:
13+
int* b;
14+
};
15+
16+
// CHECK: define dso_local void @_Z5test1P1SS0_(
17+
void test1(S* source, S* dest) {
18+
// RELOC: %0 = load ptr, ptr %dest.addr, align 8
19+
// RELOC-NEXT: %1 = load ptr, ptr %source.addr, align 8
20+
// RELOC-NEXT: call void @llvm.memmove.p0.p0.i64(ptr align 8 %0, ptr align 8 %1, i64 16, i1 false)
21+
// RELOC-NOT: @llvm.protected.field.ptr
22+
23+
// NONRELOC: %0 = load ptr, ptr %dest.addr, align 8
24+
// NONRELOC-NEXT: %1 = load ptr, ptr %source.addr, align 8
25+
// NONRELOC-NEXT: call void @llvm.memmove.p0.p0.i64(ptr align 8 %0, ptr align 8 %1, i64 16, i1 false)
26+
// NONRELOC-NEXT: br i1 false, label %loop.end, label %loop
27+
28+
// NONRELOC: loop:
29+
// NONRELOC-NEXT: %2 = phi i64 [ 0, %entry ], [ %19, %loop ]
30+
// NONRELOC-NEXT: %3 = getelementptr inbounds i8, ptr %0, i64 %2
31+
// NONRELOC-NEXT: %4 = getelementptr inbounds i8, ptr %1, i64 %2
32+
// NONRELOC-NEXT: %5 = getelementptr inbounds i8, ptr %3, i64 0
33+
// NONRELOC-NEXT: %6 = ptrtoint ptr %3 to i64
34+
// NONRELOC-NEXT: %7 = call ptr @llvm.protected.field.ptr(ptr %5, i64 %6, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.a) ]
35+
// NONRELOC-NEXT: %8 = getelementptr inbounds i8, ptr %4, i64 0
36+
// NONRELOC-NEXT: %9 = ptrtoint ptr %4 to i64
37+
// NONRELOC-NEXT: %10 = call ptr @llvm.protected.field.ptr(ptr %8, i64 %9, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.a) ]
38+
// NONRELOC-NEXT: %11 = load ptr, ptr %10, align 8
39+
// NONRELOC-NEXT: store ptr %11, ptr %7, align 8
40+
// NONRELOC-NEXT: %12 = getelementptr inbounds i8, ptr %3, i64 8
41+
// NONRELOC-NEXT: %13 = ptrtoint ptr %3 to i64
42+
// NONRELOC-NEXT: %14 = call ptr @llvm.protected.field.ptr(ptr %12, i64 %13, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.b) ]
43+
// NONRELOC-NEXT: %15 = getelementptr inbounds i8, ptr %4, i64 8
44+
// NONRELOC-NEXT: %16 = ptrtoint ptr %4 to i64
45+
// NONRELOC-NEXT: %17 = call ptr @llvm.protected.field.ptr(ptr %15, i64 %16, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.b) ]
46+
// NONRELOC-NEXT: %18 = load ptr, ptr %17, align 8
47+
// NONRELOC-NEXT: store ptr %18, ptr %14, align 8
48+
// NONRELOC-NEXT: %19 = add i64 %2, 16
49+
// NONRELOC-NEXT: %20 = icmp eq i64 %19, 16
50+
// NONRELOC-NEXT: br i1 %20, label %loop.end, label %loop
51+
52+
// NONRELOC: loop.end:
53+
// NONRELOC-NEXT: ret void
54+
__builtin_trivially_relocate(dest, source, 1);
55+
}
56+
57+
// CHECK: define dso_local void @_Z5testNP1SS0_m(
58+
void testN(S* source, S* dest, size_t count) {
59+
// RELOC: %0 = load ptr, ptr %dest.addr, align 8
60+
// RELOC-NEXT: %1 = load ptr, ptr %source.addr, align 8
61+
// RELOC-NEXT: %2 = load i64, ptr %count.addr, align 8
62+
// RELOC-NEXT: %3 = mul i64 %2, 16
63+
// RELOC-NEXT: call void @llvm.memmove.p0.p0.i64(ptr align 8 %0, ptr align 8 %1, i64 %3, i1 false)
64+
// RELOC-NOT: @llvm.protected.field.ptr
65+
66+
// NONRELOC: %0 = load ptr, ptr %dest.addr, align 8
67+
// NONRELOC-NEXT: %1 = load ptr, ptr %source.addr, align 8
68+
// NONRELOC-NEXT: %2 = load i64, ptr %count.addr, align 8
69+
// NONRELOC-NEXT: %3 = mul i64 %2, 16
70+
// NONRELOC-NEXT: call void @llvm.memmove.p0.p0.i64(ptr align 8 %0, ptr align 8 %1, i64 %3, i1 false)
71+
// NONRELOC-NEXT: %4 = icmp eq i64 %3, 0
72+
// NONRELOC-NEXT: br i1 %4, label %loop.end, label %loop
73+
74+
// NONRELOC: loop:
75+
// NONRELOC-NEXT: %5 = phi i64 [ 0, %entry ], [ %22, %loop ]
76+
// NONRELOC-NEXT: %6 = getelementptr inbounds i8, ptr %0, i64 %5
77+
// NONRELOC-NEXT: %7 = getelementptr inbounds i8, ptr %1, i64 %5
78+
// NONRELOC-NEXT: %8 = getelementptr inbounds i8, ptr %6, i64 0
79+
// NONRELOC-NEXT: %9 = ptrtoint ptr %6 to i64
80+
// NONRELOC-NEXT: %10 = call ptr @llvm.protected.field.ptr(ptr %8, i64 %9, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.a) ]
81+
// NONRELOC-NEXT: %11 = getelementptr inbounds i8, ptr %7, i64 0
82+
// NONRELOC-NEXT: %12 = ptrtoint ptr %7 to i64
83+
// NONRELOC-NEXT: %13 = call ptr @llvm.protected.field.ptr(ptr %11, i64 %12, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.a) ]
84+
// NONRELOC-NEXT: %14 = load ptr, ptr %13, align 8
85+
// NONRELOC-NEXT: store ptr %14, ptr %10, align 8
86+
// NONRELOC-NEXT: %15 = getelementptr inbounds i8, ptr %6, i64 8
87+
// NONRELOC-NEXT: %16 = ptrtoint ptr %6 to i64
88+
// NONRELOC-NEXT: %17 = call ptr @llvm.protected.field.ptr(ptr %15, i64 %16, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.b) ]
89+
// NONRELOC-NEXT: %18 = getelementptr inbounds i8, ptr %7, i64 8
90+
// NONRELOC-NEXT: %19 = ptrtoint ptr %7 to i64
91+
// NONRELOC-NEXT: %20 = call ptr @llvm.protected.field.ptr(ptr %18, i64 %19, i1 true) [ "deactivation-symbol"(ptr @__pfp_ds__ZTS1S.b) ]
92+
// NONRELOC-NEXT: %21 = load ptr, ptr %20, align 8
93+
// NONRELOC-NEXT: store ptr %21, ptr %17, align 8
94+
// NONRELOC-NEXT: %22 = add i64 %5, 16
95+
// NONRELOC-NEXT: %23 = icmp eq i64 %22, %3
96+
// NONRELOC-NEXT: br i1 %23, label %loop.end, label %loop
97+
98+
// NONRELOC: loop.end:
99+
// NONRELOC-NEXT: ret void
100+
__builtin_trivially_relocate(dest, source, count);
101+
};

0 commit comments

Comments
 (0)