[clang] b8fdafe - [Sema] Perform call checking when building CXXNewExpr
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 25 14:37:52 PST 2020
Author: Roman Lebedev
Date: 2020-02-26T01:36:44+03:00
New Revision: b8fdafe68ce25b7ff4d31720548622e28db87ebf
URL: https://github.com/llvm/llvm-project/commit/b8fdafe68ce25b7ff4d31720548622e28db87ebf
DIFF: https://github.com/llvm/llvm-project/commit/b8fdafe68ce25b7ff4d31720548622e28db87ebf.diff
LOG: [Sema] Perform call checking when building CXXNewExpr
Summary:
There was even a TODO for this.
The main motivation is to make use of call-site based
`__attribute__((alloc_align(param_idx)))` validation (D72996).
Reviewers: rsmith, erichkeane, aaron.ballman, jdoerfert
Reviewed By: rsmith
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D73020
Added:
clang/test/SemaCXX/operator-new-size-diagnose_if.cpp
Modified:
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaCXX/diagnose_if.cpp
clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 7aa115fb03c1..2a66303d6d9a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -3901,8 +3901,9 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
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();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 800454555425..9e5c7354a5eb 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2116,18 +2116,80 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
// 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);
-
- // FIXME: Missing call to CheckFunctionCall or equivalent
+ // 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.
+
+ 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.
diff --git a/clang/test/SemaCXX/diagnose_if.cpp b/clang/test/SemaCXX/diagnose_if.cpp
index 02b3620e7692..21897c5184b7 100644
--- a/clang/test/SemaCXX/diagnose_if.cpp
+++ b/clang/test/SemaCXX/diagnose_if.cpp
@@ -634,7 +634,7 @@ namespace range_for_loop {
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 @@ struct Bar {
};
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();
}
}
diff --git a/clang/test/SemaCXX/operator-new-size-diagnose_if.cpp b/clang/test/SemaCXX/operator-new-size-diagnose_if.cpp
new file mode 100644
index 000000000000..62ac2ac3a038
--- /dev/null
+++ b/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
diff --git a/clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp b/clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
index 91274ee92042..c743c15b4127 100644
--- a/clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
+++ b/clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
@@ -32,7 +32,7 @@ struct OVERALIGNED A {
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 @@ void *alloc_overaligned_struct_with_extra_256_alignment(int align) {
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
More information about the cfe-commits
mailing list