[clang] [UBSAN] add null and alignment checks for aggregates and update coverage (PR #175032)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 8 09:17:53 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: VASU SHARMA (vasu-the-sharma)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/175032.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CGExprAgg.cpp (+15-28)
- (modified) clang/lib/CodeGen/CGExprCXX.cpp (+14-15)
- (added) clang/test/CodeGen/ubsan-aggregate-null-align.c (+63)
``````````diff
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 7cc4d6c8f06f6..919e510a82af0 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -1292,45 +1292,29 @@ static bool isBlockVarRef(const Expr *E) {
void AggExprEmitter::VisitBinAssign(const BinaryOperator *E) {
ApplyAtomGroup Grp(CGF.getDebugInfo());
- // For an assignment to work, the value on the right has
- // to be compatible with the value on the left.
assert(CGF.getContext().hasSameUnqualifiedType(E->getLHS()->getType(),
E->getRHS()->getType())
- && "Invalid assignment");
+ && "Invalid assignment");
- // If the LHS might be a __block variable, and the RHS can
- // potentially cause a block copy, we need to evaluate the RHS first
- // so that the assignment goes the right place.
- // This is pretty semantically fragile.
if (isBlockVarRef(E->getLHS()) &&
E->getRHS()->HasSideEffects(CGF.getContext())) {
- // Ensure that we have a destination, and evaluate the RHS into that.
EnsureDest(E->getRHS()->getType());
Visit(E->getRHS());
-
- // Now emit the LHS and copy into it.
LValue LHS = CGF.EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
- // That copy is an atomic copy if the LHS is atomic.
if (LHS.getType()->isAtomicType() ||
CGF.LValueIsSuitableForInlineAtomic(LHS)) {
CGF.EmitAtomicStore(Dest.asRValue(), LHS, /*isInit*/ false);
return;
}
-
- EmitCopy(E->getLHS()->getType(),
- AggValueSlot::forLValue(LHS, AggValueSlot::IsDestructed,
- needsGC(E->getLHS()->getType()),
- AggValueSlot::IsAliased,
- AggValueSlot::MayOverlap),
- Dest);
+ EmitFinalDestCopy(E->getLHS()->getType(), LHS);
return;
}
- LValue LHS = CGF.EmitLValue(E->getLHS());
+ // ✅ FIX: Use EmitCheckedLValue for LHS
+ LValue LHS = CGF.EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
- // If we have an atomic type, evaluate into the destination and then
- // do an atomic copy.
+ // ✅ RE-ADD: Original atomic handling logic
if (LHS.getType()->isAtomicType() ||
CGF.LValueIsSuitableForInlineAtomic(LHS)) {
EnsureDest(E->getRHS()->getType());
@@ -1339,20 +1323,23 @@ void AggExprEmitter::VisitBinAssign(const BinaryOperator *E) {
return;
}
- // Codegen the RHS so that it stores directly into the LHS.
+ // ✅ FIX: Handle RHS based on LValue/RValue
AggValueSlot LHSSlot = AggValueSlot::forLValue(
LHS, AggValueSlot::IsDestructed, needsGC(E->getLHS()->getType()),
AggValueSlot::IsAliased, AggValueSlot::MayOverlap);
- // A non-volatile aggregate destination might have volatile member.
- if (!LHSSlot.isVolatile() &&
- CGF.hasVolatileMember(E->getLHS()->getType()))
- LHSSlot.setVolatile(true);
- CGF.EmitAggExpr(E->getRHS(), LHSSlot);
+ if (E->getRHS()->isLValue()) {
+ LValue RHS = CGF.EmitCheckedLValue(E->getRHS(), CodeGenFunction::TCK_Load);
+ CGF.EmitAggregateCopy(LHS, RHS, E->getType(), Dest.isVolatile());
+ } else {
+ if (!LHSSlot.isVolatile() && CGF.hasVolatileMember(E->getLHS()->getType()))
+ LHSSlot.setVolatile(true);
+ CGF.EmitAggExpr(E->getRHS(), LHSSlot);
+ }
- // Copy into the destination if the assignment isn't ignored.
EmitFinalDestCopy(E->getType(), LHS);
+ // ✅ RE-ADD: Original Nontrivial C struct destruction logic
if (!Dest.isIgnored() && !Dest.isExternallyDestructed() &&
E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Dest.getAddress(),
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index ce2ed9026fa1f..eae27a6a3f1c8 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -267,7 +267,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(CE)) {
if (OCE->isAssignmentOp()) {
if (TrivialAssignment) {
- TrivialAssignmentRHS = EmitLValue(CE->getArg(1));
+ TrivialAssignmentRHS = EmitCheckedLValue(CE->getArg(1), TCK_Load);
} else {
RtlArgs = &RtlArgStorage;
EmitCallArgs(*RtlArgs, MD->getType()->castAs<FunctionProtoType>(),
@@ -309,22 +309,21 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
if (TrivialForCodegen) {
if (isa<CXXDestructorDecl>(MD))
return RValue::get(nullptr);
+ }
- if (TrivialAssignment) {
- // We don't like to generate the trivial copy/move assignment operator
- // when it isn't necessary; just produce the proper effect here.
- // It's important that we use the result of EmitLValue here rather than
- // emitting call arguments, in order to preserve TBAA information from
- // the RHS.
- LValue RHS = isa<CXXOperatorCallExpr>(CE)
- ? TrivialAssignmentRHS
- : EmitLValue(*CE->arg_begin());
- EmitAggregateAssign(This, RHS, CE->getType());
- return RValue::get(This.getPointer(*this));
- }
+ if (TrivialAssignment) {
+ // 1. Evaluate 'this' (Destination) as a checked store.
+ LValue This = EmitCheckedLValue(Base, TCK_Store);
+
+ // 2. Evaluate RHS (Source) as a checked load.
+ // If it's an operator call (a = b), we use the RHS evaluated at line 270.
+ // If it's a direct call (constructor), we evaluate the first argument.
+ LValue RHS = isa<CXXOperatorCallExpr>(CE)
+ ? TrivialAssignmentRHS
+ : EmitCheckedLValue(*CE->arg_begin(), TCK_Load);
- assert(MD->getParent()->mayInsertExtraPadding() &&
- "unknown trivial member function");
+ EmitAggregateAssign(This, RHS, CE->getType());
+ return RValue::get(This.getPointer(*this));
}
// Compute the function type we're calling.
diff --git a/clang/test/CodeGen/ubsan-aggregate-null-align.c b/clang/test/CodeGen/ubsan-aggregate-null-align.c
new file mode 100644
index 0000000000000..18133327f0fc8
--- /dev/null
+++ b/clang/test/CodeGen/ubsan-aggregate-null-align.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=alignment,null \
+// RUN: -emit-llvm -std=c23 %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-UBSAN
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -std=c23 %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-NO-UBSAN
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsanitize=alignment,null \
+// RUN: -emit-llvm -xc++ %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-UBSAN
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -xc++ %s -o - \
+// RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-NO-UBSAN
+
+typedef struct Small { int x; } Small;
+typedef struct Container { struct Small inner; } Container;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// CHECK-LABEL: define {{.*}}void @test_direct_assign_ptr(
+void test_direct_assign_ptr(struct Small *dest, struct Small *src) {
+ // CHECK-UBSAN: %[[D:.*]] = load ptr, ptr %dest.addr
+ // CHECK-UBSAN: %[[S:.*]] = load ptr, ptr %src.addr
+
+ // Verify LHS (Dest) Check
+ // CHECK-UBSAN: %[[D_NULL:.*]] = icmp ne ptr %[[D]], null
+ // CHECK-UBSAN: %[[D_ALIGN:.*]] = and i64 %{{.*}}, 3
+ // CHECK-UBSAN: %[[D_ALIGN_OK:.*]] = icmp eq i64 %[[D_ALIGN]], 0
+ // CHECK-UBSAN: %[[D_OK:.*]] = and i1 %[[D_NULL]], %[[D_ALIGN_OK]]
+ // CHECK-UBSAN: br i1 %[[D_OK]], label %[[D_CONT:.*]], label %[[D_HANDLER:.*]]
+
+ // CHECK-UBSAN: [[D_HANDLER]]:
+ // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1_abort
+ // CHECK-UBSAN: unreachable
+
+ // CHECK-UBSAN: [[D_CONT]]:
+ // Verify RHS (Src) Check
+ // CHECK-UBSAN: %[[S_NULL:.*]] = icmp ne ptr %[[S]], null
+ // CHECK-UBSAN: br i1 %[[S_NULL]], label %[[S_CONT:.*]], label %[[S_HANDLER:.*]]
+
+ // CHECK-UBSAN: [[S_HANDLER]]:
+ // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1_abort
+
+ // CHECK-UBSAN: [[S_CONT]]:
+ // CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[D]], ptr align 4 %[[S]], i64 4, i1 false)
+
+ // CHECK-NO-UBSAN-NOT: @__ubsan_handle_type_mismatch
+ *dest = *src;
+}
+
+// CHECK-LABEL: define {{.*}}void @test_nested_struct(
+void test_nested_struct(struct Container *c, struct Small *s) {
+ // CHECK-UBSAN: %[[C:.*]] = load ptr, ptr %c.addr
+ // CHECK-UBSAN: icmp ne ptr %[[C]], null
+ // CHECK-UBSAN: br i1 %{{.*}}, label %[[CONT:.*]], label %[[HANDLER:.*]]
+
+ // CHECK-UBSAN: [[HANDLER]]:
+ // CHECK-UBSAN: call void @__ubsan_handle_type_mismatch_v1_abort
+ c->inner = *s;
+}
+
+#ifdef __cplusplus
+}
+#endif
``````````
</details>
https://github.com/llvm/llvm-project/pull/175032
More information about the cfe-commits
mailing list