From 243d82345d3471c5375d57e8ac186327e4c82211 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 13 Nov 2012 05:07:23 +0000 Subject: [PATCH] Copy the decls returned by DeclContext::lookup_result to a new container so we can safely iterate over them. The container holding the lookup decls can under certain conditions be changed while iterating (e.g. because of deserialization). llvm-svn: 167816 --- clang/lib/Sema/SemaInit.cpp | 39 ++++++++++++++++++++++++++------------ clang/test/PCH/crash-12631281.cpp | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 clang/test/PCH/crash-12631281.cpp diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 3596bbf..7ed3942 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2744,14 +2744,14 @@ static OverloadingResult ResolveConstructorOverload(Sema &S, SourceLocation DeclLoc, Expr **Args, unsigned NumArgs, OverloadCandidateSet &CandidateSet, - DeclContext::lookup_iterator Con, - DeclContext::lookup_iterator ConEnd, + ArrayRef Ctors, OverloadCandidateSet::iterator &Best, bool CopyInitializing, bool AllowExplicit, bool OnlyListConstructors, bool InitListSyntax) { CandidateSet.clear(); - for (; Con != ConEnd; ++Con) { + for (ArrayRef::iterator + Con = Ctors.begin(), ConEnd = Ctors.end(); Con != ConEnd; ++Con) { NamedDecl *D = *Con; DeclAccessPair FoundDecl = DeclAccessPair::make(D, D->getAccess()); bool SuppressUserConversions = false; @@ -2844,6 +2844,10 @@ static void TryConstructorInitialization(Sema &S, // through overload resolution. DeclContext::lookup_iterator ConStart, ConEnd; llvm::tie(ConStart, ConEnd) = S.LookupConstructors(DestRecordDecl); + // The container holding the constructors can under certain conditions + // be changed while iterating (e.g. because of deserialization). + // To be safe we copy the lookup results to a new container. + SmallVector Ctors(ConStart, ConEnd); OverloadingResult Result = OR_No_Viable_Function; OverloadCandidateSet::iterator Best; @@ -2865,7 +2869,7 @@ static void TryConstructorInitialization(Sema &S, (!DestRecordDecl->hasDeclaredDefaultConstructor() && !DestRecordDecl->needsImplicitDefaultConstructor())) Result = ResolveConstructorOverload(S, Kind.getLocation(), Args, NumArgs, - CandidateSet, ConStart, ConEnd, Best, + CandidateSet, Ctors, Best, CopyInitialization, AllowExplicit, /*OnlyListConstructor=*/true, InitListSyntax); @@ -2883,7 +2887,7 @@ static void TryConstructorInitialization(Sema &S, if (Result == OR_No_Viable_Function) { AsInitializerList = false; Result = ResolveConstructorOverload(S, Kind.getLocation(), Args, NumArgs, - CandidateSet, ConStart, ConEnd, Best, + CandidateSet, Ctors, Best, CopyInitialization, AllowExplicit, /*OnlyListConstructors=*/false, InitListSyntax); @@ -3153,9 +3157,14 @@ static OverloadingResult TryRefInitWithConversionFunction(Sema &S, CXXRecordDecl *T1RecordDecl = cast(T1RecordType->getDecl()); DeclContext::lookup_iterator Con, ConEnd; - for (llvm::tie(Con, ConEnd) = S.LookupConstructors(T1RecordDecl); - Con != ConEnd; ++Con) { - NamedDecl *D = *Con; + llvm::tie(Con, ConEnd) = S.LookupConstructors(T1RecordDecl); + // The container holding the constructors can under certain conditions + // be changed while iterating (e.g. because of deserialization). + // To be safe we copy the lookup results to a new container. + SmallVector Ctors(Con, ConEnd); + for (SmallVector::iterator + CI = Ctors.begin(), CE = Ctors.end(); CI != CE; ++CI) { + NamedDecl *D = *CI; DeclAccessPair FoundDecl = DeclAccessPair::make(D, D->getAccess()); // Find the constructor (which may be a template). @@ -4317,11 +4326,17 @@ static void LookupCopyAndMoveConstructors(Sema &S, CXXRecordDecl *Class, Expr *CurInitExpr) { DeclContext::lookup_iterator Con, ConEnd; - for (llvm::tie(Con, ConEnd) = S.LookupConstructors(Class); - Con != ConEnd; ++Con) { + llvm::tie(Con, ConEnd) = S.LookupConstructors(Class); + // The container holding the constructors can under certain conditions + // be changed while iterating (e.g. because of deserialization). + // To be safe we copy the lookup results to a new container. + SmallVector Ctors(Con, ConEnd); + for (SmallVector::iterator + CI = Ctors.begin(), CE = Ctors.end(); CI != CE; ++CI) { + NamedDecl *D = *CI; CXXConstructorDecl *Constructor = 0; - if ((Constructor = dyn_cast(*Con))) { + if ((Constructor = dyn_cast(D))) { // Handle copy/moveconstructors, only. if (!Constructor || Constructor->isInvalidDecl() || !Constructor->isCopyOrMoveConstructor() || @@ -4336,7 +4351,7 @@ static void LookupCopyAndMoveConstructors(Sema &S, } // Handle constructor templates. - FunctionTemplateDecl *ConstructorTmpl = cast(*Con); + FunctionTemplateDecl *ConstructorTmpl = cast(D); if (ConstructorTmpl->isInvalidDecl()) continue; diff --git a/clang/test/PCH/crash-12631281.cpp b/clang/test/PCH/crash-12631281.cpp new file mode 100644 index 0000000..f309bca --- /dev/null +++ b/clang/test/PCH/crash-12631281.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 %s -emit-pch -o %t.pch +// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -include-pch %t.pch -verify +// expected-no-diagnostics + +// rdar://12631281 +// This reduced test case exposed a use-after-free memory bug, which was reliable +// reproduced only on guarded malloc (and probably valgrind). + +#ifndef HEADER +#define HEADER + +template < class _T2> struct is_convertible; +template <> struct is_convertible { typedef int type; }; + +template struct pair { + typedef _T1 first_type; + typedef _T2 second_type; + template ::type> + pair(_U1&& , _U2&& ); // expected-note {{candidate}} +}; + +template +pair<_ForwardIterator, _ForwardIterator> __equal_range(_ForwardIterator) { + return pair<_ForwardIterator, _ForwardIterator>(0, 0); // expected-error {{no matching constructor}} +} + +template +pair<_ForwardIterator, _ForwardIterator> equal_range( _ForwardIterator a) { + return __equal_range(a); // expected-note {{instantiation}} +} + +class A { + pair range() { + return equal_range(0); // expected-note {{instantiation}} + } +}; + +#else + +#endif -- 2.7.4