https://github.com/vasu-the-sharma created https://github.com/llvm/llvm-project/pull/175032
None >From f10bbba299bfcda6ac69af7aa3b7e11107484d72 Mon Sep 17 00:00:00 2001 From: vasu-ibm <[email protected]> Date: Thu, 8 Jan 2026 11:49:39 -0500 Subject: [PATCH 1/2] add coverage ubsan-aggregate-null-align.c --- .../test/CodeGen/ubsan-aggregate-null-align.c | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 clang/test/CodeGen/ubsan-aggregate-null-align.c 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..7ca9d32c3305b --- /dev/null +++ b/clang/test/CodeGen/ubsan-aggregate-null-align.c @@ -0,0 +1,48 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \ +// RUN: -fsanitize=null,alignment | FileCheck %s --check-prefix=CHECK-SANITIZE +// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \ +// RUN: | FileCheck %s --check-prefix=CHECK-NO-SANITIZE + +struct Small { int x; }; +struct Container { struct Small inner; }; + +// CHECK-SANITIZE-LABEL: define {{.*}}void @test_direct_assign_ptr( +// CHECK-SANITIZE: %[[D:.*]] = load ptr, ptr %dest.addr +// CHECK-SANITIZE: %[[S:.*]] = load ptr, ptr %src.addr +// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[D]], ptr align 4 %[[S]], i64 4, i1 false) +// +// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_direct_assign_ptr( +// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +void test_direct_assign_ptr(struct Small *dest, struct Small *src) { + *dest = *src; +} + +// CHECK-SANITIZE-LABEL: define {{.*}}void @test_null_dest( +// CHECK-SANITIZE: %[[D:.*]] = load ptr, ptr %dest +// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[D]], ptr {{.*}}, i64 4, i1 false) +// +// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_null_dest( +// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +void test_null_dest(struct Small *src) { + struct Small *dest = 0; + *dest = *src; +} + +// CHECK-SANITIZE-LABEL: define {{.*}}void @test_nested_struct( +// CHECK-SANITIZE: %[[VAL1:.*]] = icmp ne ptr %[[C:.*]], null +// CHECK-SANITIZE: br i1 %{{.*}}, label %cont, label %handler.type_mismatch +// +// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_nested_struct( +// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +void test_nested_struct(struct Container *c, struct Small *s) { + c->inner = *s; +} + +// CHECK-SANITIZE-LABEL: define {{.*}}void @test_comma_operator( +// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %{{.*}}, ptr align 4 %{{.*}}, i64 4, i1 false) +// +// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_comma_operator( +// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +void test_comma_operator(struct Small *dest, struct Small *src) { + *dest = (0, *src); +} >From cc64e3f111e790760b1c624fa27ae03701f1d021 Mon Sep 17 00:00:00 2001 From: vasu-ibm <[email protected]> Date: Thu, 8 Jan 2026 12:15:28 -0500 Subject: [PATCH 2/2] add null and alignment checks for aggregates --- clang/lib/CodeGen/CGExprAgg.cpp | 43 +++------ clang/lib/CodeGen/CGExprCXX.cpp | 29 +++--- .../test/CodeGen/ubsan-aggregate-null-align.c | 91 +++++++++++-------- 3 files changed, 82 insertions(+), 81 deletions(-) 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 index 7ca9d32c3305b..18133327f0fc8 100644 --- a/clang/test/CodeGen/ubsan-aggregate-null-align.c +++ b/clang/test/CodeGen/ubsan-aggregate-null-align.c @@ -1,48 +1,63 @@ -// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \ -// RUN: -fsanitize=null,alignment | FileCheck %s --check-prefix=CHECK-SANITIZE -// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \ -// RUN: | FileCheck %s --check-prefix=CHECK-NO-SANITIZE - -struct Small { int x; }; -struct Container { struct Small inner; }; - -// CHECK-SANITIZE-LABEL: define {{.*}}void @test_direct_assign_ptr( -// CHECK-SANITIZE: %[[D:.*]] = load ptr, ptr %dest.addr -// CHECK-SANITIZE: %[[S:.*]] = load ptr, ptr %src.addr -// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[D]], ptr align 4 %[[S]], i64 4, i1 false) -// -// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_direct_assign_ptr( -// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +// 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) { - *dest = *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-SANITIZE-LABEL: define {{.*}}void @test_null_dest( -// CHECK-SANITIZE: %[[D:.*]] = load ptr, ptr %dest -// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[D]], ptr {{.*}}, i64 4, i1 false) -// -// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_null_dest( -// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch -void test_null_dest(struct Small *src) { - struct Small *dest = 0; + // 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-SANITIZE-LABEL: define {{.*}}void @test_nested_struct( -// CHECK-SANITIZE: %[[VAL1:.*]] = icmp ne ptr %[[C:.*]], null -// CHECK-SANITIZE: br i1 %{{.*}}, label %cont, label %handler.type_mismatch -// -// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_nested_struct( -// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch +// 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; } -// CHECK-SANITIZE-LABEL: define {{.*}}void @test_comma_operator( -// CHECK-SANITIZE: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %{{.*}}, ptr align 4 %{{.*}}, i64 4, i1 false) -// -// CHECK-NO-SANITIZE-LABEL: define {{.*}}void @test_comma_operator( -// CHECK-NO-SANITIZE-NOT: @__ubsan_handle_type_mismatch -void test_comma_operator(struct Small *dest, struct Small *src) { - *dest = (0, *src); +#ifdef __cplusplus } +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
