https://github.com/hubert-reinterpretcast updated https://github.com/llvm/llvm-project/pull/203737
>From 5131f000c9f37ab6b90f9e093b948aee94485921 Mon Sep 17 00:00:00 2001 From: Hubert Tong <[email protected]> Date: Sat, 13 Jun 2026 21:11:46 -0400 Subject: [PATCH 1/2] [UBSan] Use EmitCheckedLValue for C++ trivial operator= operands Further to https://github.com/llvm/llvm-project/pull/190739, use EmitCheckedLValue for trivial operator= operands * for the LHS (`lhs->` not handled yet), and * for the RHS also for function call syntax. --- clang/lib/CodeGen/CGExprCXX.cpp | 43 ++++++++----- .../ubsan-aggregate-null-align-bounds.c | 63 ++++++++++++------- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 0dc2e0bb82114..ebbc0addfed2c 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -277,22 +277,27 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( } } - LValue This; - if (IsArrow) { - LValueBaseInfo BaseInfo; - TBAAAccessInfo TBAAInfo; - Address ThisValue = EmitPointerWithAlignment(Base, &BaseInfo, &TBAAInfo); - This = MakeAddrLValue(ThisValue, Base->getType()->getPointeeType(), - BaseInfo, TBAAInfo); - } else { - This = EmitLValue(Base); - } + auto getLValueForThis = [this, IsArrow, + Base](bool EmitCheckedForStore = false) { + // FIXME: Respect EmitCheckedForStore for the IsArrow case. + if (IsArrow) { + LValueBaseInfo BaseInfo; + TBAAAccessInfo TBAAInfo; + Address ThisValue = EmitPointerWithAlignment(Base, &BaseInfo, &TBAAInfo); + return MakeAddrLValue(ThisValue, Base->getType()->getPointeeType(), + BaseInfo, TBAAInfo); + } + if (EmitCheckedForStore) + return EmitCheckedLValue(Base, TCK_Store); + return EmitLValue(Base); + }; if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) { // This is the MSVC p->Ctor::Ctor(...) extension. We assume that's // constructing a new complete object of type Ctor. assert(!RtlArgs); assert(ReturnValue.isNull() && "Constructor shouldn't have return value"); + LValue This = getLValueForThis(); CallArgList Args; commonEmitCXXMemberOrOperatorCall( *this, {Ctor, Ctor_Complete}, This.getPointer(*this), @@ -307,17 +312,22 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( } if (TrivialForCodegen) { - if (isa<CXXDestructorDecl>(MD)) + if (isa<CXXDestructorDecl>(MD)) { + (void)getLValueForThis(); // Emit LHS for side effects. 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()); + LValue This = getLValueForThis(/*EmitCheckedForStore=*/true); + + // It's important that we use the result of EmitCheckedLValue here rather + // than emitting call arguments, in order to preserve TBAA information + // from the RHS. + LValue RHS = isa<CXXOperatorCallExpr>(CE) + ? TrivialAssignmentRHS + : EmitCheckedLValue(*CE->arg_begin(), TCK_Load); EmitAggregateAssign(This, RHS, CE->getType()); return RValue::get(This.getPointer(*this)); } @@ -356,6 +366,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( SkippedChecks.set(SanitizerKind::Null, true); } + LValue This = getLValueForThis(); if (sanitizePerformTypeCheck()) EmitTypeCheck(CodeGenFunction::TCK_MemberCall, CallLoc, This.emitRawPointer(*this), diff --git a/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c b/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c index 9fc3fd6e64584..c855f0a845d0b 100644 --- a/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c +++ b/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c @@ -15,29 +15,26 @@ struct Agg { int x; }; extern "C" { #endif -// LHS checks - C only -// Note: In C++, aggregate assignment goes through operator= -// which is a different code path (CGExprCXX.cpp). -// FIXME: LHS checks for C++ will be addressed in a follow-up PR - -// C-LABEL: define {{.*}}@test_lhs_ptrcheck_deref( -// C: [[DEST:%.*]] = load ptr, ptr %dest.addr -// C-NEXT: [[CMP:%.*]] = icmp ne ptr [[DEST]], null, !nosanitize -// C-NEXT: [[INT:%.*]] = ptrtoint ptr [[DEST]] to i64, !nosanitize -// C-NEXT: [[AND:%.*]] = and i64 [[INT]], 3, !nosanitize -// C-NEXT: [[ALIGN:%.*]] = icmp eq i64 [[AND]], 0, !nosanitize -// C-NEXT: [[OK:%.*]] = and i1 [[CMP]], [[ALIGN]], !nosanitize -// C-NEXT: br i1 [[OK]], label %cont, label %handler.type_mismatch -// C: handler.type_mismatch: -// C-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort -// C: call void @llvm.memcpy +// LHS checks - both C and C++ + +// CHECK-LABEL: define {{.*}}@test_lhs_ptrcheck_deref( +// CHECK: [[DEST:%.*]] = load ptr, ptr %dest.addr +// CHECK-NEXT: [[CMP:%.*]] = icmp ne ptr [[DEST]], null, !nosanitize +// CHECK-NEXT: [[INT:%.*]] = ptrtoint ptr [[DEST]] to i64, !nosanitize +// CHECK-NEXT: [[AND:%.*]] = and i64 [[INT]], 3, !nosanitize +// CHECK-NEXT: [[ALIGN:%.*]] = icmp eq i64 [[AND]], 0, !nosanitize +// CHECK-NEXT: [[OK:%.*]] = and i1 [[CMP]], [[ALIGN]], !nosanitize +// CHECK-NEXT: br i1 [[OK]], label %cont, label %handler.type_mismatch +// CHECK: handler.type_mismatch: +// CHECK-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK: call void @llvm.memcpy void test_lhs_ptrcheck_deref(AGG *dest) { AGG local = {0}; *dest = local; } -// C-LABEL: define {{.*}}@test_lhs_ptrcheck_subscript( -// C: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK-LABEL: define {{.*}}@test_lhs_ptrcheck_subscript( +// CHECK: call void @__ubsan_handle_type_mismatch_v1_abort void test_lhs_ptrcheck_subscript(AGG arr[4]) { AGG local = {0}; arr[0] = local; @@ -88,6 +85,8 @@ void test_init_from_subscript(AGG arr[4]) { } // Array bounds - out-of-bounds access (RHS) +// Note: GCC also does not detect the out-of-bounds access here when compiled as +// C++. // CHECK-LABEL: define {{.*}}@test_oob_rhs( // C: br i1 false, label %cont, label %handler.out_of_bounds @@ -105,15 +104,13 @@ void test_oob_rhs(void) { } // Array bounds - out-of-bounds access (LHS) -// FIXME: LHS checks for C++ will be addressed in a follow-up PR. // CHECK-LABEL: define {{.*}}@test_oob_lhs( -// C: br i1 false, label %cont, label %handler.out_of_bounds -// CXX: br i1 true, label %cont, label %handler.out_of_bounds +// CHECK: br i1 false, label %cont, label %handler.out_of_bounds // CHECK: handler.out_of_bounds: // CHECK-NEXT: call void @__ubsan_handle_out_of_bounds_abort -// C: handler.type_mismatch: -// C-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK: handler.type_mismatch: +// CHECK-NEXT: call void @__ubsan_handle_type_mismatch_v1_abort // CHECK: call void @llvm.memcpy void test_oob_lhs(void) { AGG arr[4]; @@ -126,12 +123,30 @@ void test_oob_lhs(void) { } #endif -// C++ RHS cases - handler call only +// C++ cases - handler call only #ifdef __cplusplus extern "C" { +// C++ LHS cases + +// CXX-LABEL: define {{.*}}@test_cxx_lhs_dot_operator_function_call( +// CXX: call void @__ubsan_handle_type_mismatch_v1_abort +void test_cxx_lhs_dot_operator_function_call(AGG *src) { + AGG aggValue(void); + (*src).operator=(aggValue()); +} + +// C++ RHS cases + +// CXX-LABEL: define {{.*}}@test_cxx_rhs_operator_function_call( +// CXX: call void @__ubsan_handle_type_mismatch_v1_abort +void test_cxx_rhs_operator_function_call(AGG *src) { + AGG local = {0}; + local.operator=(*src); +} + // CXX-LABEL: define {{.*}}@test_cxx_direct_init( // CXX: call void @__ubsan_handle_type_mismatch_v1_abort void test_cxx_direct_init(AGG *src) { >From d131e00011ed50ad5282d6a82a2a69c0234c38b1 Mon Sep 17 00:00:00 2001 From: Hubert Tong <[email protected]> Date: Thu, 18 Jun 2026 23:22:53 -0400 Subject: [PATCH 2/2] Strengthen handler-call-only checks against extra handler calls --- clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c b/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c index c855f0a845d0b..7fd2e6c2d0300 100644 --- a/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c +++ b/clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c @@ -35,6 +35,7 @@ void test_lhs_ptrcheck_deref(AGG *dest) { // CHECK-LABEL: define {{.*}}@test_lhs_ptrcheck_subscript( // CHECK: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_lhs_ptrcheck_subscript(AGG arr[4]) { AGG local = {0}; arr[0] = local; @@ -62,6 +63,7 @@ void test_rhs_ptrcheck_deref(AGG *src) { // CHECK-LABEL: define {{.*}}@test_rhs_ptrcheck_subscript( // CHECK: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_rhs_ptrcheck_subscript(AGG arr[4]) { AGG local; local = arr[0]; @@ -72,6 +74,7 @@ void test_rhs_ptrcheck_subscript(AGG arr[4]) { // CHECK-LABEL: define {{.*}}@test_init_from_deref( // CHECK: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_init_from_deref(AGG *src) { AGG local = *src; (void)local; @@ -79,6 +82,7 @@ void test_init_from_deref(AGG *src) { // CHECK-LABEL: define {{.*}}@test_init_from_subscript( // CHECK: call void @__ubsan_handle_type_mismatch_v1_abort +// CHECK-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_init_from_subscript(AGG arr[4]) { AGG local = arr[0]; (void)local; @@ -133,6 +137,7 @@ extern "C" { // CXX-LABEL: define {{.*}}@test_cxx_lhs_dot_operator_function_call( // CXX: call void @__ubsan_handle_type_mismatch_v1_abort +// CXX-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_cxx_lhs_dot_operator_function_call(AGG *src) { AGG aggValue(void); (*src).operator=(aggValue()); @@ -142,6 +147,7 @@ void test_cxx_lhs_dot_operator_function_call(AGG *src) { // CXX-LABEL: define {{.*}}@test_cxx_rhs_operator_function_call( // CXX: call void @__ubsan_handle_type_mismatch_v1_abort +// CXX-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_cxx_rhs_operator_function_call(AGG *src) { AGG local = {0}; local.operator=(*src); @@ -149,6 +155,7 @@ void test_cxx_rhs_operator_function_call(AGG *src) { // CXX-LABEL: define {{.*}}@test_cxx_direct_init( // CXX: call void @__ubsan_handle_type_mismatch_v1_abort +// CXX-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_cxx_direct_init(AGG *src) { AGG local(*src); (void)local; @@ -156,6 +163,7 @@ void test_cxx_direct_init(AGG *src) { // CXX-LABEL: define {{.*}}@test_cxx_brace_init( // CXX: call void @__ubsan_handle_type_mismatch_v1_abort +// CXX-NOT: call void @__ubsan_handle_type_mismatch_v1_abort void test_cxx_brace_init(AGG *src) { AGG local{*src}; (void)local; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
