llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Hubert Tong (hubert-reinterpretcast) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/203737.diff 2 Files Affected: - (modified) clang/lib/CodeGen/CGExprCXX.cpp (+27-16) - (modified) clang/test/CodeGen/ubsan-aggregate-null-align-bounds.c (+39-24) ``````````diff 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) { `````````` </details> https://github.com/llvm/llvm-project/pull/203737 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
