From: Richard Smith Date: Tue, 13 May 2014 19:56:21 +0000 (+0000) Subject: PR19729: Delete a bunch of bogus code in Sema::FindAllocationOverload. This X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f579a0e9261498db6cd09f0b8a438c6b30bc3218;p=clang PR19729: Delete a bunch of bogus code in Sema::FindAllocationOverload. This caused us to perform copy-initialization for the parameters of an allocation function called by a new-expression multiple times, resulting in us rejecting allocations that passed non-copyable parameters (and much worse things in MSVC compat mode, where we potentially called this function multiple times). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@208724 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 3d90f0a655..33eea164a1 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -4164,18 +4164,14 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl, VariadicCallType CallType, bool AllowExplicit, bool IsListInitialization) { unsigned NumParams = Proto->getNumParams(); - unsigned NumArgsToCheck = Args.size(); bool Invalid = false; - if (Args.size() != NumParams) - // Use default arguments for missing arguments - NumArgsToCheck = NumParams; unsigned ArgIx = 0; // Continue to check argument types (even if we have too few/many args). - for (unsigned i = FirstParam; i != NumArgsToCheck; i++) { + for (unsigned i = FirstParam; i < NumParams; i++) { QualType ProtoArgType = Proto->getParamType(i); Expr *Arg; - ParmVarDecl *Param; + ParmVarDecl *Param = FDecl ? FDecl->getParamDecl(i) : nullptr; if (ArgIx < Args.size()) { Arg = Args[ArgIx++]; @@ -4184,11 +4180,6 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl, diag::err_call_incomplete_argument, Arg)) return true; - // Pass the argument - Param = 0; - if (FDecl && i < FDecl->getNumParams()) - Param = FDecl->getParamDecl(i); - // Strip the unbridged-cast placeholder expression off, if applicable. bool CFAudited = false; if (Arg->getType() == Context.ARCUnbridgedCastTy && @@ -4209,7 +4200,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl, // Remember that parameter belongs to a CF audited API. if (CFAudited) Entity.setParameterCFAudited(); - + ExprResult ArgE = PerformCopyInitialization(Entity, SourceLocation(), Owned(Arg), @@ -4220,8 +4211,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl, Arg = ArgE.takeAs(); } else { - assert(FDecl && "can't use default arguments without a known callee"); - Param = FDecl->getParamDecl(i); + assert(Param && "can't use default arguments without a known callee"); ExprResult ArgExpr = BuildCXXDefaultArgExpr(CallLoc, FDecl, Param); diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 6f60406a4e..20916668ef 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -1414,12 +1414,14 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, SmallVector AllPlaceArgs; if (OperatorNew) { - // Add default arguments, if any. const FunctionProtoType *Proto = - OperatorNew->getType()->getAs(); - VariadicCallType CallType = - Proto->isVariadic() ? VariadicFunction : VariadicDoesNotApply; + OperatorNew->getType()->getAs(); + VariadicCallType CallType = Proto->isVariadic() ? VariadicFunction + : VariadicDoesNotApply; + // We've already converted the placement args, just fill in any default + // arguments. Skip the first parameter because we don't have a corresponding + // argument. if (GatherArgumentsForCall(PlacementLParen, OperatorNew, Proto, 1, PlacementArgs, AllPlaceArgs, CallType)) return ExprError(); @@ -1427,6 +1429,7 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal, 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 @@ -1684,11 +1687,6 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, return false; } - // FindAllocationOverload can change the passed in arguments, so we need to - // copy them back. - if (!PlaceArgs.empty()) - std::copy(AllocArgs.begin() + 1, AllocArgs.end(), PlaceArgs.data()); - // C++ [expr.new]p19: // // If the new-expression begins with a unary :: operator, the @@ -1832,8 +1830,22 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, return false; } -/// FindAllocationOverload - Find an fitting overload for the allocation -/// function in the specified scope. +/// \brief Find an fitting overload for the allocation function +/// in the specified scope. +/// +/// \param StartLoc The location of the 'new' token. +/// \param SourceRange The range of the placement arguments. +/// \param Name The name of the function ('operator new' or 'operator new[]'). +/// \param Args The placement arguments specified. +/// \param Ctx The scope in which we should search; either a class scope or the +/// translation unit. +/// \param AllowMissing If \c true, report an error if we can't find any +/// allocation functions. Otherwise, succeed but don't fill in \p +/// Operator. +/// \param Operator Filled in with the found allocation function. Unchanged if +/// no allocation function was found. +/// \param Diagnose If \c true, issue errors if the allocation function is not +/// usable. bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range, DeclarationName Name, MultiExprArg Args, DeclContext *Ctx, @@ -1879,33 +1891,11 @@ bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range, case OR_Success: { // Got one! FunctionDecl *FnDecl = Best->Function; - MarkFunctionReferenced(StartLoc, FnDecl); - // The first argument is size_t, and the first parameter must be size_t, - // too. This is checked on declaration and can be assumed. (It can't be - // asserted on, though, since invalid decls are left in there.) - // Watch out for variadic allocator function. - unsigned NumArgsInFnDecl = FnDecl->getNumParams(); - for (unsigned i = 0; (i < Args.size() && i < NumArgsInFnDecl); ++i) { - InitializedEntity Entity = InitializedEntity::InitializeParameter(Context, - FnDecl->getParamDecl(i)); - - if (!Diagnose && !CanPerformCopyInitialization(Entity, Owned(Args[i]))) - return true; - - ExprResult Result - = PerformCopyInitialization(Entity, SourceLocation(), Owned(Args[i])); - if (Result.isInvalid()) - return true; - - Args[i] = Result.takeAs(); - } - - Operator = FnDecl; - if (CheckAllocationAccess(StartLoc, Range, R.getNamingClass(), Best->FoundDecl, Diagnose) == AR_inaccessible) return true; + Operator = FnDecl; return false; } diff --git a/test/SemaCXX/cxx0x-initializer-constructor.cpp b/test/SemaCXX/cxx0x-initializer-constructor.cpp index dc179f81bd..2b443fce78 100644 --- a/test/SemaCXX/cxx0x-initializer-constructor.cpp +++ b/test/SemaCXX/cxx0x-initializer-constructor.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++0x -fsyntax-only -verify %s +// RUN: %clang_cc1 -std=c++0x -fsyntax-only -fexceptions -verify %s struct one { char c[1]; }; struct two { char c[2]; }; @@ -304,7 +304,6 @@ namespace init_list_default { B b {}; // calls default constructor } - // PR13470, namespace PR13470 { struct W { @@ -365,3 +364,14 @@ namespace PR13470 { yi.h(); // ok, all diagnostics produced in template definition } } + +namespace PR19729 { + struct A { + A(int); + A(const A&) = delete; + }; + struct B { + void *operator new(std::size_t, A); + }; + B *p = new ({123}) B; +} diff --git a/test/SemaCXX/microsoft-new-delete.cpp b/test/SemaCXX/microsoft-new-delete.cpp index e0d25dcd86..6c9be22893 100644 --- a/test/SemaCXX/microsoft-new-delete.cpp +++ b/test/SemaCXX/microsoft-new-delete.cpp @@ -1,5 +1,4 @@ -// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify %s -// expected-no-diagnostics +// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify -std=c++11 %s #include @@ -10,3 +9,26 @@ void f() { // Expect no error in MSVC compatibility mode int *p = new(arbitrary) int[4]; } + +class noncopyable { noncopyable(const noncopyable&); } extern nc; // expected-note {{here}} +void *operator new[](size_t, noncopyable); +void *operator new(size_t, const noncopyable&); +void *q = new (nc) int[4]; // expected-error {{calling a private constructor}} + +struct bitfield { int n : 3; } bf; // expected-note {{here}} +void *operator new[](size_t, int &); +void *operator new(size_t, const int &); +void *r = new (bf.n) int[4]; // expected-error {{non-const reference cannot bind to bit-field}} + +struct base {}; +struct derived : private base {} der; // expected-note {{here}} +void *operator new[](size_t, base &); +void *operator new(size_t, derived &); +void *s = new (der) int[4]; // expected-error {{private}} + +struct explicit_ctor { explicit explicit_ctor(int); }; +struct explicit_ctor_tag {} ect; +void *operator new[](size_t, explicit_ctor_tag, explicit_ctor); +void *operator new(size_t, explicit_ctor_tag, int); +void *t = new (ect, 0) int[4]; +void *u = new (ect, {0}) int[4];