lebedev.ri updated this revision to Diff 245625. lebedev.ri added a comment.
Rebased ontop of patch demoting call-site-based align attr checking from error into a warning, NFC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73020/new/ https://reviews.llvm.org/D73020 Files: clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaExprCXX.cpp clang/test/SemaCXX/diagnose_if.cpp clang/test/SemaCXX/operator-new-size-diagnose_if.cpp clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
Index: clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp =================================================================== --- clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp +++ clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp @@ -32,7 +32,7 @@ void *ptr_variable(int align) { return new (std::align_val_t(align)) A; } void *ptr_align16() { return new (std::align_val_t(16)) A; } -void *ptr_align15() { return new (std::align_val_t(15)) A; } +void *ptr_align15() { return new (std::align_val_t(15)) A; } // expected-warning {{requested alignment is not a power of 2}} struct alignas(128) S { S() {} @@ -49,11 +49,9 @@ return new (std::align_val_t(256)) S; } void *alloc_overaligned_struct_with_extra_255_alignment(int align) { - return new (std::align_val_t(255)) S; + return new (std::align_val_t(255)) S; // expected-warning {{requested alignment is not a power of 2}} } std::align_val_t align_variable(int align) { return std::align_val_t(align); } std::align_val_t align_align16() { return std::align_val_t(16); } std::align_val_t align_align15() { return std::align_val_t(15); } - -// expected-no-diagnostics Index: clang/test/SemaCXX/operator-new-size-diagnose_if.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/operator-new-size-diagnose_if.cpp @@ -0,0 +1,24 @@ +// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++14 + +using size_t = decltype(sizeof(int)); + +#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__))) + +namespace operator_new { +struct T0 { + int j = 0; + static void *operator new(size_t i) _diagnose_if(i == sizeof(int), "yay", "warning"); // expected-note{{from 'diagnose_if'}} +}; + +struct T1 { + int j = 0; + static void *operator new[](size_t i) _diagnose_if(i == 8 * sizeof(int), "yay", "warning"); // expected-note 2{{from 'diagnose_if'}} +}; + +void run(int x) { + new T0; // expected-warning{{yay}} + new T1[8]; // expected-warning{{yay}} + new T1[4][2]; // expected-warning{{yay}} + new T1[x]; // no warning. +} +} // namespace operator_new Index: clang/test/SemaCXX/diagnose_if.cpp =================================================================== --- clang/test/SemaCXX/diagnose_if.cpp +++ clang/test/SemaCXX/diagnose_if.cpp @@ -634,7 +634,7 @@ namespace operator_new { struct Foo { int j; - static void *operator new(size_t i) _diagnose_if(i, "oh no", "warning"); + static void *operator new(size_t i) _diagnose_if(i, "oh no", "warning"); // expected-note{{from 'diagnose_if'}} }; struct Bar { @@ -643,10 +643,7 @@ }; void run() { - // FIXME: This should emit a diagnostic. - new Foo(); - // This is here because we sometimes pass a dummy argument `operator new`. We - // should ignore this, rather than complaining about it. + new Foo(); // expected-warning{{oh no}} new Bar(); } } Index: clang/lib/Sema/SemaExprCXX.cpp =================================================================== --- clang/lib/Sema/SemaExprCXX.cpp +++ clang/lib/Sema/SemaExprCXX.cpp @@ -2116,18 +2116,80 @@ // arguments. Skip the first parameter because we don't have a corresponding // argument. Skip the second parameter too if we're passing in the // alignment; we've already filled it in. + unsigned NumImplicitArgs = PassAlignment ? 2 : 1; if (GatherArgumentsForCall(PlacementLParen, OperatorNew, Proto, - PassAlignment ? 2 : 1, PlacementArgs, - AllPlaceArgs, CallType)) + NumImplicitArgs, PlacementArgs, AllPlaceArgs, + CallType)) return ExprError(); if (!AllPlaceArgs.empty()) PlacementArgs = AllPlaceArgs; - // FIXME: This is wrong: PlacementArgs misses out the first (size) argument. - DiagnoseSentinelCalls(OperatorNew, PlacementLParen, PlacementArgs); + // We would like to perform some checking on the given `operator new` call, + // but the PlacementArgs does not contain the implicit arguments, + // namely allocation size and maybe allocation alignment, + // so we need to conjure them. - // FIXME: Missing call to CheckFunctionCall or equivalent + QualType SizeTy = Context.getSizeType(); + unsigned SizeTyWidth = Context.getTypeSize(SizeTy); + + llvm::APInt SingleEltSize( + SizeTyWidth, Context.getTypeSizeInChars(AllocType).getQuantity()); + + // How many bytes do we want to allocate here? + llvm::Optional<llvm::APInt> AllocationSize; + if (!ArraySize.hasValue() && !AllocType->isDependentType()) { + // For non-array operator new, we only want to allocate one element. + AllocationSize = SingleEltSize; + } else if (KnownArraySize.hasValue() && !AllocType->isDependentType()) { + // For array operator new, only deal with static array size case. + bool Overflow; + AllocationSize = llvm::APInt(SizeTyWidth, *KnownArraySize) + .umul_ov(SingleEltSize, Overflow); + (void)Overflow; + assert( + !Overflow && + "Expected that all the overflows would have been handled already."); + } + + IntegerLiteral AllocationSizeLiteral( + Context, + AllocationSize.getValueOr(llvm::APInt::getNullValue(SizeTyWidth)), + SizeTy, SourceLocation()); + // Otherwise, if we failed to constant-fold the allocation size, we'll + // just give up and pass-in something opaque, that isn't a null pointer. + OpaqueValueExpr OpaqueAllocationSize(SourceLocation(), SizeTy, VK_RValue, + OK_Ordinary, /*SourceExpr=*/nullptr); + + // Let's synthesize the alignment argument in case we will need it. + // Since we *really* want to allocate these on stack, this is slightly ugly + // because there might not be a `std::align_val_t` type. + EnumDecl *StdAlignValT = getStdAlignValT(); + QualType AlignValT = + StdAlignValT ? Context.getTypeDeclType(StdAlignValT) : SizeTy; + IntegerLiteral AlignmentLiteral( + Context, + llvm::APInt(Context.getTypeSize(SizeTy), + Alignment / Context.getCharWidth()), + SizeTy, SourceLocation()); + ImplicitCastExpr DesiredAlignment(ImplicitCastExpr::OnStack, AlignValT, + CK_IntegralCast, &AlignmentLiteral, + VK_RValue); + + // Adjust placement args by prepending conjured size and alignment exprs. + llvm::SmallVector<Expr *, 8> CallArgs; + CallArgs.reserve(NumImplicitArgs + PlacementArgs.size()); + CallArgs.emplace_back(AllocationSize.hasValue() + ? static_cast<Expr *>(&AllocationSizeLiteral) + : &OpaqueAllocationSize); + if (PassAlignment) + CallArgs.emplace_back(&DesiredAlignment); + CallArgs.insert(CallArgs.end(), PlacementArgs.begin(), PlacementArgs.end()); + + DiagnoseSentinelCalls(OperatorNew, PlacementLParen, CallArgs); + + checkCall(OperatorNew, Proto, /*ThisArg=*/nullptr, CallArgs, + /*IsMemberFunction=*/false, StartLoc, Range, CallType); // Warn if the type is over-aligned and is being allocated by (unaligned) // global operator new. Index: clang/lib/Sema/SemaChecking.cpp =================================================================== --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -3896,8 +3896,9 @@ auto *AA = FDecl->getAttr<AllocAlignAttr>(); const Expr *Arg = Args[AA->getParamIndex().getASTIndex()]; if (!Arg->isValueDependent()) { - llvm::APSInt I(64); - if (Arg->isIntegerConstantExpr(I, Context)) { + Expr::EvalResult Align; + if (Arg->EvaluateAsInt(Align, Context)) { + const llvm::APSInt &I = Align.Val.getInt(); if (!I.isPowerOf2()) Diag(Arg->getExprLoc(), diag::warn_alignment_not_power_of_two) << Arg->getSourceRange();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits