Author: Markus Böck Date: 2022-10-24T21:41:13+02:00 New Revision: 3637dc601c4923721a69426187aa69dd6a71a053
URL: https://github.com/llvm/llvm-project/commit/3637dc601c4923721a69426187aa69dd6a71a053 DIFF: https://github.com/llvm/llvm-project/commit/3637dc601c4923721a69426187aa69dd6a71a053.diff LOG: [clang][CodeGen] Consistently return nullptr Values for void builtins and scalar initalization A common post condition of the various visitor functions in CodeGen is that instructions, that do not return any values, simply return a nullptr Value as a sentinel. This has not been the case however for calls to some builtins returning void, as well as for an initializer expression of the form `void()`. This would then lead to ICEs in CodeGen on code relying on nullptr being returned for void values, which is eg. the case for conditional expressions [0]. This patch fixes that by returning nullptr Values for intrinsics known not to return any values as well as for a scalar initializer returning void. Fixes https://github.com/llvm/llvm-project/issues/53127 [0] https://github.com/llvm/llvm-project/blob/266ec801fb23f9f5f1d61ca9466e0805fbdb78a7/clang/lib/CodeGen/CGExprScalar.cpp#L4849-L4892 Differential Revision: https://reviews.llvm.org/D136548 Added: clang/test/CodeGen/pr53127.cpp Modified: clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGExprScalar.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index fbb6e85e37d6e..f69b1e80607f8 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -2518,11 +2518,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, case Builtin::BI__builtin_va_start: case Builtin::BI__va_start: case Builtin::BI__builtin_va_end: - return RValue::get( - EmitVAStartEnd(BuiltinID == Builtin::BI__va_start - ? EmitScalarExpr(E->getArg(0)) - : EmitVAListRef(E->getArg(0)).getPointer(), - BuiltinID != Builtin::BI__builtin_va_end)); + EmitVAStartEnd(BuiltinID == Builtin::BI__va_start + ? EmitScalarExpr(E->getArg(0)) + : EmitVAListRef(E->getArg(0)).getPointer(), + BuiltinID != Builtin::BI__builtin_va_end); + return RValue::get(nullptr); case Builtin::BI__builtin_va_copy: { Value *DstPtr = EmitVAListRef(E->getArg(0)).getPointer(); Value *SrcPtr = EmitVAListRef(E->getArg(1)).getPointer(); @@ -2531,8 +2531,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, DstPtr = Builder.CreateBitCast(DstPtr, Type); SrcPtr = Builder.CreateBitCast(SrcPtr, Type); - return RValue::get(Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy), - {DstPtr, SrcPtr})); + Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy), {DstPtr, SrcPtr}); + return RValue::get(nullptr); } case Builtin::BI__builtin_abs: case Builtin::BI__builtin_labs: @@ -2804,7 +2804,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Value *ArgValue = EmitScalarExpr(E->getArg(0)); Function *FnAssume = CGM.getIntrinsic(Intrinsic::assume); - return RValue::get(Builder.CreateCall(FnAssume, ArgValue)); + Builder.CreateCall(FnAssume, ArgValue); + return RValue::get(nullptr); } case Builtin::BI__arithmetic_fence: { // Create the builtin call if FastMath is selected, and the target @@ -2925,7 +2926,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, llvm::ConstantInt::get(Int32Ty, 3); Value *Data = llvm::ConstantInt::get(Int32Ty, 1); Function *F = CGM.getIntrinsic(Intrinsic::prefetch, Address->getType()); - return RValue::get(Builder.CreateCall(F, {Address, RW, Locality, Data})); + Builder.CreateCall(F, {Address, RW, Locality, Data}); + return RValue::get(nullptr); } case Builtin::BI__builtin_readcyclecounter: { Function *F = CGM.getIntrinsic(Intrinsic::readcyclecounter); @@ -2938,9 +2940,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, return RValue::get(Builder.CreateCall(F, {Begin, End})); } case Builtin::BI__builtin_trap: - return RValue::get(EmitTrapCall(Intrinsic::trap)); + EmitTrapCall(Intrinsic::trap); + return RValue::get(nullptr); case Builtin::BI__debugbreak: - return RValue::get(EmitTrapCall(Intrinsic::debugtrap)); + EmitTrapCall(Intrinsic::debugtrap); + return RValue::get(nullptr); case Builtin::BI__builtin_unreachable: { EmitUnreachable(E->getExprLoc()); @@ -3721,7 +3725,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, } case Builtin::BI__builtin_unwind_init: { Function *F = CGM.getIntrinsic(Intrinsic::eh_unwind_init); - return RValue::get(Builder.CreateCall(F)); + Builder.CreateCall(F); + return RValue::get(nullptr); } case Builtin::BI__builtin_extend_pointer: { // Extends a pointer to the size of an _Unwind_Word, which is @@ -4482,8 +4487,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, return EmitBuiltinNewDeleteCall( E->getCallee()->getType()->castAs<FunctionProtoType>(), E, false); case Builtin::BI__builtin_operator_delete: - return EmitBuiltinNewDeleteCall( + EmitBuiltinNewDeleteCall( E->getCallee()->getType()->castAs<FunctionProtoType>(), E, true); + return RValue::get(nullptr); case Builtin::BI__builtin_is_aligned: return EmitBuiltinIsAligned(E); @@ -4653,7 +4659,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, case Builtin::BI__builtin_coro_promise: return EmitCoroutineIntrinsic(E, Intrinsic::coro_promise); case Builtin::BI__builtin_coro_resume: - return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume); + EmitCoroutineIntrinsic(E, Intrinsic::coro_resume); + return RValue::get(nullptr); case Builtin::BI__builtin_coro_frame: return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame); case Builtin::BI__builtin_coro_noop: @@ -4661,7 +4668,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, case Builtin::BI__builtin_coro_free: return EmitCoroutineIntrinsic(E, Intrinsic::coro_free); case Builtin::BI__builtin_coro_destroy: - return EmitCoroutineIntrinsic(E, Intrinsic::coro_destroy); + EmitCoroutineIntrinsic(E, Intrinsic::coro_destroy); + return RValue::get(nullptr); case Builtin::BI__builtin_coro_done: return EmitCoroutineIntrinsic(E, Intrinsic::coro_done); case Builtin::BI__builtin_coro_alloc: @@ -5094,7 +5102,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, Value *Val = EmitScalarExpr(E->getArg(0)); Address Address = EmitPointerWithAlignment(E->getArg(1)); Value *HalfVal = Builder.CreateFPTrunc(Val, Builder.getHalfTy()); - return RValue::get(Builder.CreateStore(HalfVal, Address)); + Builder.CreateStore(HalfVal, Address); + return RValue::get(nullptr); } case Builtin::BI__builtin_load_half: { Address Address = EmitPointerWithAlignment(E->getArg(0)); @@ -5359,6 +5368,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, V = Builder.CreateBitCast(V, RetTy); } + if (RetTy->isVoidTy()) + return RValue::get(nullptr); + return RValue::get(V); } @@ -5376,6 +5388,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID, if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue)) { switch (EvalKind) { case TEK_Scalar: + if (V->getType()->isVoidTy()) + return RValue::get(nullptr); return RValue::get(V); case TEK_Aggregate: return RValue::getAggregate(ReturnValue.getValue(), diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index eaaeb052aaef4..7b1337b1be68c 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -467,6 +467,9 @@ class ScalarExprEmitter return llvm::ConstantInt::get(ConvertType(E->getType()), E->getValue()); } Value *VisitCXXScalarValueInitExpr(const CXXScalarValueInitExpr *E) { + if (E->getType()->isVoidType()) + return nullptr; + return EmitNullValue(E->getType()); } Value *VisitGNUNullExpr(const GNUNullExpr *E) { diff --git a/clang/test/CodeGen/pr53127.cpp b/clang/test/CodeGen/pr53127.cpp new file mode 100644 index 0000000000000..97fe1291352d3 --- /dev/null +++ b/clang/test/CodeGen/pr53127.cpp @@ -0,0 +1,87 @@ +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py +// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu | FileCheck %s + +bool e(); + +void operator delete(void*); + +// CHECK-LABEL: @_Z1fPiz( +// CHECK-NEXT: entry: +// CHECK-NEXT: [[P_ADDR:%.*]] = alloca ptr, align 8 +// CHECK-NEXT: [[L:%.*]] = alloca [1 x %struct.__va_list_tag], align 16 +// CHECK-NEXT: [[L2:%.*]] = alloca [1 x %struct.__va_list_tag], align 16 +// CHECK-NEXT: store ptr [[P:%.*]], ptr [[P_ADDR]], align 8 +// CHECK-NEXT: [[CALL:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]] +// CHECK: cond.true: +// CHECK-NEXT: call void @llvm.trap() +// CHECK-NEXT: br label [[COND_END:%.*]] +// CHECK: cond.false: +// CHECK-NEXT: br label [[COND_END]] +// CHECK: cond.end: +// CHECK-NEXT: [[CALL1:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL1]], label [[COND_TRUE2:%.*]], label [[COND_FALSE3:%.*]] +// CHECK: cond.true2: +// CHECK-NEXT: call void @llvm.debugtrap() +// CHECK-NEXT: br label [[COND_END4:%.*]] +// CHECK: cond.false3: +// CHECK-NEXT: br label [[COND_END4]] +// CHECK: cond.end4: +// CHECK-NEXT: [[CALL5:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: [[TMP0:%.*]] = zext i1 [[CALL5]] to i64 +// CHECK-NEXT: call void @llvm.assume(i1 true) +// CHECK-NEXT: [[CALL6:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL6]], label [[COND_TRUE7:%.*]], label [[COND_FALSE8:%.*]] +// CHECK: cond.true7: +// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L]], i64 0, i64 0 +// CHECK-NEXT: call void @llvm.va_start(ptr [[ARRAYDECAY]]) +// CHECK-NEXT: br label [[COND_END9:%.*]] +// CHECK: cond.false8: +// CHECK-NEXT: br label [[COND_END9]] +// CHECK: cond.end9: +// CHECK-NEXT: [[CALL10:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL10]], label [[COND_TRUE11:%.*]], label [[COND_FALSE14:%.*]] +// CHECK: cond.true11: +// CHECK-NEXT: [[ARRAYDECAY12:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L]], i64 0, i64 0 +// CHECK-NEXT: [[ARRAYDECAY13:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L2]], i64 0, i64 0 +// CHECK-NEXT: call void @llvm.va_copy(ptr [[ARRAYDECAY12]], ptr [[ARRAYDECAY13]]) +// CHECK-NEXT: br label [[COND_END15:%.*]] +// CHECK: cond.false14: +// CHECK-NEXT: br label [[COND_END15]] +// CHECK: cond.end15: +// CHECK-NEXT: [[CALL16:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL16]], label [[COND_TRUE17:%.*]], label [[COND_FALSE18:%.*]] +// CHECK: cond.true17: +// CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: call void @llvm.prefetch.p0(ptr [[TMP1]], i32 0, i32 3, i32 1) +// CHECK-NEXT: br label [[COND_END19:%.*]] +// CHECK: cond.false18: +// CHECK-NEXT: br label [[COND_END19]] +// CHECK: cond.end19: +// CHECK-NEXT: [[CALL20:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: br i1 [[CALL20]], label [[COND_TRUE21:%.*]], label [[COND_FALSE22:%.*]] +// CHECK: cond.true21: +// CHECK-NEXT: call void @llvm.eh.unwind.init() +// CHECK-NEXT: br label [[COND_END23:%.*]] +// CHECK: cond.false22: +// CHECK-NEXT: br label [[COND_END23]] +// CHECK: cond.end23: +// CHECK-NEXT: [[CALL24:%.*]] = call noundef zeroext i1 @_Z1ev() +// CHECK-NEXT: [[TMP2:%.*]] = zext i1 [[CALL24]] to i64 +// CHECK-NEXT: [[TMP3:%.*]] = load ptr, ptr [[P_ADDR]], align 8 +// CHECK-NEXT: call void @_ZdlPv(ptr noundef [[TMP3]]) #[[ATTR8:[0-9]+]] +// CHECK-NEXT: ret void +// +void f(int* p, ...) +{ + e() ? __builtin_trap() : void(); + e() ? __builtin_debugtrap() : void(); + e() ? __builtin_assume(true) : void(); + __builtin_va_list l; + e() ? __builtin_va_start(l, p) : void(); + __builtin_va_list l2; + e() ? __builtin_va_copy(l, l2) : void(); + e() ? __builtin_prefetch(p) : void(); + e() ? __builtin_unwind_init() : void(); + e() ? __builtin_operator_delete(p) : void(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits