Skip to content

Commit

Permalink
[Sema] Perform call checking when building CXXNewExpr
Browse files Browse the repository at this point in the history
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
  • Loading branch information
LebedevRI committed Feb 25, 2020
1 parent 6d2372c commit b8fdafe
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 17 deletions.
5 changes: 3 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
Expand Up @@ -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();
Expand Down
74 changes: 68 additions & 6 deletions clang/lib/Sema/SemaExprCXX.cpp
Expand Up @@ -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.
Expand Down
7 changes: 2 additions & 5 deletions clang/test/SemaCXX/diagnose_if.cpp
Expand Up @@ -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 {
Expand All @@ -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();
}
}
Expand Down
24 changes: 24 additions & 0 deletions 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
6 changes: 2 additions & 4 deletions clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp
Expand Up @@ -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() {}
Expand All @@ -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

0 comments on commit b8fdafe

Please sign in to comment.