Skip to content

Add pointer field protection feature. #133538

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 5 commits into
base: users/pcc/spr/main.add-pointer-field-protection-feature
Choose a base branch
from
Open

Add pointer field protection feature. #133538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3bdbe71
[𝘀𝗽𝗿] initial version
pcc Mar 28, 2025
40b2f77
Rebase
pcc May 13, 2025
0d82cb3
Rebase, extract trivially relocatable inhibition condition into a fun…
pcc May 24, 2025
50ab633
Rebase
pcc Jul 9, 2025
5b1e91a
Rebase
pcc Jul 10, 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
22 changes: 22 additions & 0 deletions clang/include/clang/AST/ASTContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,12 @@ struct TypeInfoChars {
}
};

struct PFPField {
CharUnits structOffset;
CharUnits offset;
FieldDecl *field;
};

/// Holds long-lived AST nodes (such as types and decls) that can be
/// referred to throughout the semantic analysis of a file.
class ASTContext : public RefCountedBase<ASTContext> {
Expand Down Expand Up @@ -3703,6 +3709,22 @@ OPT_LIST(V)

StringRef getCUIDHash() const;

bool isPFPStruct(const RecordDecl *rec) const;
void findPFPFields(QualType Ty, CharUnits Offset,
std::vector<PFPField> &Fields, bool IncludeVBases) const;
bool hasPFPFields(QualType ty) const;
bool isPFPField(const FieldDecl *field) const;

/// Returns whether this record's PFP fields (if any) are trivially
/// relocatable (i.e. may be memcpy'd). This may also return true if the
/// record does not have any PFP fields, so it may be necessary for the caller
/// to check for PFP fields, e.g. by calling hasPFPFields().
bool arePFPFieldsTriviallyRelocatable(const RecordDecl *RD) const;

llvm::SetVector<const FieldDecl *> PFPFieldsWithEvaluatedOffset;
void recordMemberDataPointerEvaluation(const ValueDecl *VD);
void recordOffsetOfEvaluation(const OffsetOfExpr *E);

private:
/// All OMPTraitInfo objects live in this collection, one per
/// `pragma omp [begin] declare variant` directive.
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2597,6 +2597,12 @@ def CountedByOrNull : DeclOrTypeAttr {
let LangOpts = [COnly];
}

def NoPointerFieldProtection : DeclOrTypeAttr {
Copy link
Contributor

Choose a reason for hiding this comment

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

This an ABI break so I don't think it can reasonably an on by default for all structs - we can already see annotations in libc++, and they would be needed on every single struct field.

We can imagine a new platform where this would be the platform ABI, but for every other case it is functionally unusable.

I would recommend attributes to permit opt in and opt out on a per-struct basis, and CLI flags to select the default behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are numerous circumstances where the C++ ABI is distinct from the platform ABI and is allowed to change. For example, most Chromium-based web browsers ship with a statically linked libc++, and the same is true for many Android apps and server-side software at Google. In the intended usage model, all of libc++ would be built with a -fexperimental-pointer-field-protection= flag consistent with the rest of the program.

The libc++abi opt-outs that I added are only for opting out pointer fields of RTTI structs that are generated by the compiler. In principle, we could teach the compiler to sign those pointer fields which would let us remove the opt-outs, but there is a lower ROI for subjecting those fields to PFP because the RTTI structs are not typically dynamically allocated.

We may consider adding an attribute to allow opting in in the case where the flag is not passed. But I think we should do that in a followup. We would need to carefully consider how that would interact with the other aspects of the opt-in solution, such as the pointer qualifiers.

let Spellings = [Clang<"no_field_protection">];
let Subjects = SubjectList<[Field], ErrorDiag>;
let Documentation = [Undocumented];
}

def SizedBy : DeclOrTypeAttr {
let Spellings = [Clang<"sized_by">];
let Subjects = SubjectList<[Field], ErrorDiag>;
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/Features.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ FEATURE(shadow_call_stack,
FEATURE(tls, PP.getTargetInfo().isTLSSupported())
FEATURE(underlying_type, LangOpts.CPlusPlus)
FEATURE(experimental_library, LangOpts.ExperimentalLibrary)
FEATURE(pointer_field_protection,
LangOpts.getPointerFieldProtection() !=
LangOptions::PointerFieldProtectionKind::None)

// C11 features supported by other languages as extensions.
EXTENSION(c_alignas, true)
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/LangOptions.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,9 @@ LANGOPT(RelativeCXXABIVTables, 1, 0, NotCompatible,
LANGOPT(OmitVTableRTTI, 1, 0, NotCompatible,
"Use an ABI-incompatible v-table layout that omits the RTTI component")

ENUM_LANGOPT(PointerFieldProtection, PointerFieldProtectionKind, 2, PointerFieldProtectionKind::None, NotCompatible,
"Encode struct pointer fields to protect against UAF vulnerabilities")

LANGOPT(VScaleMin, 32, 0, NotCompatible, "Minimum vscale value")
LANGOPT(VScaleMax, 32, 0, NotCompatible, "Maximum vscale value")

Expand Down
11 changes: 11 additions & 0 deletions clang/include/clang/Basic/LangOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,17 @@ class LangOptionsBase {
BKey
};

enum class PointerFieldProtectionKind {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I like this being solely a global decision - it makes custom allocators much harder, and it makes it hard for allocators that have different policies, e.g

struct ImportantStruct {
   void *operator new(size_t) { /*tagged allocator*/ }
 };

struct BoringStruct {
   ...
};

struct SomeOtherStruct {
  ImportantStruct *important; // should be tagged pointer
  BoringString *lessImportant; // should be a non tagged pointer
};

This would again require a struct attribute to indicate the pointer policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that allowing this level of customization should be implemented as part of the separate opt-in solution (e.g. it may be a property of the qualifier). It is generally a reasonable assumption that operator new is the same for all types. In the unlikely case that it isn't, we aren't really doing anything wrong by using the malloc-derived decision here. It just means that we lose some entropy from the type or the pointer tag.

/// Pointer field protection disabled
None,
/// Pointer field protection enabled, allocator does not tag heap
/// allocations.
Untagged,
/// Pointer field protection enabled, allocator is expected to tag heap
/// allocations.
Tagged,
};

enum class ThreadModelKind {
/// POSIX Threads.
POSIX,
Expand Down
6 changes: 6 additions & 0 deletions clang/include/clang/Driver/Options.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3033,6 +3033,12 @@ defm experimental_omit_vtable_rtti : BoolFOption<"experimental-omit-vtable-rtti"
NegFlag<SetFalse, [], [CC1Option], "Do not omit">,
BothFlags<[], [CC1Option], " the RTTI component from virtual tables">>;

def experimental_pointer_field_protection_EQ : Joined<["-"], "fexperimental-pointer-field-protection=">, Group<f_Group>,
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, not super sure that I like this being a global compile time setting, but maybe it makes sense to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(See other reply.)

Visibility<[ClangOption, CC1Option]>,
Values<"none,untagged,tagged">, NormalizedValuesScope<"LangOptions::PointerFieldProtectionKind">,
NormalizedValues<["None", "Untagged", "Tagged"]>,
MarshallingInfoEnum<LangOpts<"PointerFieldProtection">, "None">;

def fcxx_abi_EQ : Joined<["-"], "fc++-abi=">,
Group<f_clang_Group>, Visibility<[ClangOption, CC1Option]>,
HelpText<"C++ ABI to use. This will override the target C++ ABI.">;
Expand Down
94 changes: 94 additions & 0 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15162,3 +15162,97 @@ bool ASTContext::useAbbreviatedThunkName(GlobalDecl VirtualMethodDecl,
ThunksToBeAbbreviated[VirtualMethodDecl] = std::move(SimplifiedThunkNames);
return Result;
}

bool ASTContext::arePFPFieldsTriviallyRelocatable(const RecordDecl *RD) const {
if (getLangOpts().getPointerFieldProtection() ==
LangOptions::PointerFieldProtectionKind::Tagged)
return !isa<CXXRecordDecl>(RD) ||
cast<CXXRecordDecl>(RD)->hasTrivialDestructor();
return true;
}

bool ASTContext::isPFPStruct(const RecordDecl *rec) const {
if (getLangOpts().getPointerFieldProtection() !=
LangOptions::PointerFieldProtectionKind::None)
if (auto *cxxRec = dyn_cast<CXXRecordDecl>(rec))
return !cxxRec->isStandardLayout();
return false;
}

void ASTContext::findPFPFields(QualType Ty, CharUnits Offset,
std::vector<PFPField> &Fields,
bool IncludeVBases) const {
if (auto *AT = getAsConstantArrayType(Ty)) {
if (auto *ElemDecl = AT->getElementType()->getAsCXXRecordDecl()) {
const ASTRecordLayout &ElemRL = getASTRecordLayout(ElemDecl);
for (unsigned i = 0; i != AT->getSize(); ++i) {
findPFPFields(AT->getElementType(), Offset + i * ElemRL.getSize(),
Fields, true);
}
}
}
auto *Decl = Ty->getAsCXXRecordDecl();
if (!Decl)
return;
const ASTRecordLayout &RL = getASTRecordLayout(Decl);
for (FieldDecl *field : Decl->fields()) {
CharUnits fieldOffset =
Offset + toCharUnitsFromBits(RL.getFieldOffset(field->getFieldIndex()));
if (isPFPField(field))
Fields.push_back({Offset, fieldOffset, field});
findPFPFields(field->getType(), fieldOffset, Fields, true);
}
for (auto &Base : Decl->bases()) {
if (Base.isVirtual())
continue;
CharUnits BaseOffset =
Offset + RL.getBaseClassOffset(Base.getType()->getAsCXXRecordDecl());
findPFPFields(Base.getType(), BaseOffset, Fields, false);
}
if (IncludeVBases) {
for (auto &Base : Decl->vbases()) {
CharUnits BaseOffset =
Offset + RL.getVBaseClassOffset(Base.getType()->getAsCXXRecordDecl());
findPFPFields(Base.getType(), BaseOffset, Fields, false);
}
}
}

bool ASTContext::hasPFPFields(QualType ty) const {
std::vector<PFPField> pfpFields;
findPFPFields(ty, CharUnits::Zero(), pfpFields, true);
return !pfpFields.empty();
}

bool ASTContext::isPFPField(const FieldDecl *field) const {
if (!isPFPStruct(field->getParent()))
return false;
return field->getType()->isPointerType() &&
!field->hasAttr<NoPointerFieldProtectionAttr>();
}

void ASTContext::recordMemberDataPointerEvaluation(const ValueDecl *VD) {
if (getLangOpts().getPointerFieldProtection() ==
LangOptions::PointerFieldProtectionKind::None)
return;
auto *FD = dyn_cast<FieldDecl>(VD);
if (!FD)
FD = cast<FieldDecl>(cast<IndirectFieldDecl>(VD)->chain().back());
if (!isPFPField(FD))
return;
PFPFieldsWithEvaluatedOffset.insert(FD);
}

void ASTContext::recordOffsetOfEvaluation(const OffsetOfExpr *E) {
if (getLangOpts().getPointerFieldProtection() ==
LangOptions::PointerFieldProtectionKind::None ||
E->getNumComponents() == 0)
return;
OffsetOfNode Comp = E->getComponent(E->getNumComponents() - 1);
if (Comp.getKind() != OffsetOfNode::Field)
return;
FieldDecl *FD = Comp.getField();
if (!isPFPField(FD))
return;
PFPFieldsWithEvaluatedOffset.insert(FD);
}
1 change: 1 addition & 0 deletions clang/lib/AST/ExprConstant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15001,6 +15001,7 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
}

bool IntExprEvaluator::VisitOffsetOfExpr(const OffsetOfExpr *OOE) {
Info.Ctx.recordOffsetOfEvaluation(OOE);
CharUnits Result;
unsigned n = OOE->getNumComponents();
if (n == 0)
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/AST/TypePrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2119,6 +2119,9 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
case attr::ExtVectorType:
OS << "ext_vector_type";
break;
case attr::NoPointerFieldProtection:
OS << "no_field_protection";
break;
}
OS << "))";
}
Expand Down
13 changes: 13 additions & 0 deletions clang/lib/CodeGen/CGBuiltin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4494,6 +4494,19 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
EmitArgCheck(TCK_Load, Src, E->getArg(1), 1);
auto *I = Builder.CreateMemMove(Dest, Src, SizeVal, false);
addInstToNewSourceAtom(I, nullptr);
if (BuiltinIDIfNoAsmLabel == Builtin::BI__builtin_trivially_relocate) {
std::vector<PFPField> PFPFields;
getContext().findPFPFields(E->getArg(0)->getType()->getPointeeType(),
CharUnits::Zero(), PFPFields, true);
for (auto &Field : PFPFields) {
if (getContext().arePFPFieldsTriviallyRelocatable(
Field.field->getParent()))
continue;
auto DestFieldPtr = EmitAddressOfPFPField(Dest, Field);
auto SrcFieldPtr = EmitAddressOfPFPField(Src, Field);
Builder.CreateStore(Builder.CreateLoad(SrcFieldPtr), DestFieldPtr);
}
}
return RValue::get(Dest, *this);
}
case Builtin::BImemset:
Expand Down
Loading
Loading