[PATCH] Fix missed exception spec checks and crashes
Alp Toker
alp at nuanti.com
Sun Oct 6 11:54:27 PDT 2013
This patch fixes a crash and rejects-valid the libcxx developers were
hitting.
While investigating the defaulted member cases I noticed a similar
problem in delayed destructor checking, with checks accumulating but
never running or getting cleared if they were instantiated within an
exception spec.
I'll appreciate thoughts on both parts of the fix, particularly on the
correctness of moving the destructor checks from
ActOnFinishCXXMemberDecls() to ActOnFinishDelayedMemberInitializers()
alongside the member checks, which seems to be the only way to get them
handled consistently.
Marshall, can you think of anything else to excercise in the test case?
I've attached the patch and crash backtrace. Here's the changelog:
Delayed exception specification checking for defaulted members and
virtual destructors are both susceptible to mutation during iteration so
we need to swap and process the worklists iteratively.
This resolves both accepts-invalid and rejects-valid issues and moreover
fixes potential invalid memory access as the contents of the vectors
change during iteration and recursive template instantiation.
This patch also adds two assertions at end of TU to ensure no specs are
left unchecked as was happenning before the fix, plus a test case from
Marshall Clow for the defaulted member crash extracted from the libcxx
headers.
Alp.
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
commit 6745489f5c2ce990792eeef1f95c00e9221c0573
Author: Alp Toker <alp at nuanti.com>
Date: Thu Oct 3 07:18:14 2013 +0100
Fix missed exception spec checks and crashes
Delayed exception specification checking for defaulted members and virtual
destructors are both susceptible to mutation during iteration so we need to
swap and process the worklists iteratively.
This resolves both accepts-invalid and rejects-valid issues and moreover fixes
potential invalid memory access as the contents of the vectors change during
iteration and recursive template instantiation.
This patch also adds two assertions at end of TU to ensure no specs are left
unchecked as was happenning before the fix, plus a test case from Marshall Clow
for the defaulted member crash extracted from the libcxx headers.
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 6ba53d5..120f0a3 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -4782,7 +4782,7 @@ public:
void CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD);
void CheckExplicitlyDefaultedMemberExceptionSpec(CXXMethodDecl *MD,
const FunctionProtoType *T);
- void CheckDelayedExplicitlyDefaultedMemberExceptionSpecs();
+ void CheckDelayedMemberExceptionSpecs();
//===--------------------------------------------------------------------===//
// C++ Derived Classes
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 6f2b309..4fb9ffb 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -792,6 +792,11 @@ void Sema::ActOnEndOfTranslationUnit() {
}
}
+ // All delayed member exception specs should be checked or we end up accepting
+ // incompatible declarations.
+ assert(DelayedDefaultedMemberExceptionSpecs.empty());
+ assert(DelayedDestructorExceptionSpecChecks.empty());
+
// Check we've noticed that we're no longer parsing the initializer for every
// variable. If we miss cases, then at best we have a performance issue and
// at worst a rejects-valid bug.
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 518f315..0b52ebb 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -4831,14 +4831,33 @@ void Sema::CheckExplicitlyDefaultedMemberExceptionSpec(
SpecifiedType, MD->getLocation());
}
-void Sema::CheckDelayedExplicitlyDefaultedMemberExceptionSpecs() {
- for (unsigned I = 0, N = DelayedDefaultedMemberExceptionSpecs.size();
- I != N; ++I)
- CheckExplicitlyDefaultedMemberExceptionSpec(
- DelayedDefaultedMemberExceptionSpecs[I].first,
- DelayedDefaultedMemberExceptionSpecs[I].second);
+void Sema::CheckDelayedMemberExceptionSpecs() {
+ while (true) {
+ SmallVector
+ <std::pair<const CXXDestructorDecl *, const CXXDestructorDecl *>, 2> Checks;
+ SmallVector<std::pair<CXXMethodDecl *, const FunctionProtoType *>, 2> Specs;
+
+ std::swap(Checks, DelayedDestructorExceptionSpecChecks);
+ std::swap(Specs, DelayedDefaultedMemberExceptionSpecs);
+
+ if (Checks.empty() && Specs.empty())
+ break;
+
+ // Perform any deferred checking of exception specifications for virtual
+ // destructors.
+ for (unsigned i = 0, e = Checks.size(); i != e; ++i) {
+ const CXXDestructorDecl *Dtor = Checks[i].first;
+ assert(!Dtor->getParent()->isDependentType() &&
+ "Should not ever add destructors of templates into the list.");
+ CheckOverridingFunctionExceptionSpec(Dtor, Checks[i].second);
+ }
- DelayedDefaultedMemberExceptionSpecs.clear();
+ // Check that any explicitly-defaulted methods have exception specifications
+ // compatible with their implicit exception specifications.
+ for (unsigned I = 0, N = Specs.size(); I != N; ++I)
+ CheckExplicitlyDefaultedMemberExceptionSpec(Specs[I].first,
+ Specs[I].second);
+ }
}
namespace {
@@ -8149,9 +8168,8 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,
}
void Sema::ActOnFinishDelayedMemberInitializers(Decl *D) {
- // Check that any explicitly-defaulted methods have exception specifications
- // compatible with their implicit exception specifications.
- CheckDelayedExplicitlyDefaultedMemberExceptionSpecs();
+ // Perform any delayed checks on exception specifications.
+ CheckDelayedMemberExceptionSpecs();
// Once all the member initializers are processed, perform checks to see if
// any unintialized use is happeneing.
@@ -8674,23 +8692,11 @@ void Sema::ActOnFinishCXXMemberDecls() {
// If the context is an invalid C++ class, just suppress these checks.
if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(CurContext)) {
if (Record->isInvalidDecl()) {
+ DelayedDefaultedMemberExceptionSpecs.clear();
DelayedDestructorExceptionSpecChecks.clear();
return;
}
}
-
- // Perform any deferred checking of exception specifications for virtual
- // destructors.
- for (unsigned i = 0, e = DelayedDestructorExceptionSpecChecks.size();
- i != e; ++i) {
- const CXXDestructorDecl *Dtor =
- DelayedDestructorExceptionSpecChecks[i].first;
- assert(!Dtor->getParent()->isDependentType() &&
- "Should not ever add destructors of templates into the list.");
- CheckOverridingFunctionExceptionSpec(Dtor,
- DelayedDestructorExceptionSpecChecks[i].second);
- }
- DelayedDestructorExceptionSpecChecks.clear();
}
void Sema::AdjustDestructorExceptionSpec(CXXRecordDecl *ClassDecl,
diff --git a/test/SemaTemplate/exception-spec-crash.cpp b/test/SemaTemplate/exception-spec-crash.cpp
new file mode 100644
index 0000000..4d93559
--- /dev/null
+++ b/test/SemaTemplate/exception-spec-crash.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fcxx-exceptions -DCXX_EXCEPTIONS -fsyntax-only -verify %s
+
+template <class _Tp> struct is_nothrow_move_constructible {
+ static const bool value = false;
+};
+
+template <class _Tp>
+class allocator;
+
+template <>
+class allocator<char> {};
+
+template <class _Allocator>
+class basic_string {
+ typedef _Allocator allocator_type;
+ basic_string(basic_string &&__str)
+ noexcept(is_nothrow_move_constructible<allocator_type>::value);
+};
+
+class Foo {
+ Foo(Foo &&) noexcept = default;
+#ifdef CXX_EXCEPTIONS
+// expected-error at -2 {{does not match the calculated}}
+#else
+// expected-no-diagnostics
+#endif
+ Foo &operator=(Foo &&) noexcept = default;
+ basic_string<allocator<char> > vectorFoo_;
+};
-------------- next part --------------
Assertion failed: (begin() + idx < end()), function operator[], file /Users/alp/Projects/llvm-work/upstream/llvm/include/llvm/ADT/SmallVector.h, line 140.
0 clang 0x000000010c9856c5 llvm::sys::PrintStackTrace(__sFILE*) + 40
1 clang 0x000000010c985acc SignalHandler(int) + 241
2 libsystem_c.dylib 0x00007fff8c82490a _sigtramp + 26
3 clang 0x000000010c961114 llvm::FoldingSetNodeID::AddInteger(int) + 20
4 clang 0x000000010c985944 abort + 22
5 clang 0x000000010c98592e abort + 0
6 clang 0x000000010ce73bda llvm::SmallVectorTemplateCommon<std::pair<clang::CXXMethodDecl*, clang::FunctionProtoType const*>, void>::operator[](unsigned int) + 52
7 clang 0x000000010ce50f6e clang::Sema::CheckDelayedExplicitlyDefaultedMemberExceptionSpecs() + 62
8 clang 0x000000010ce59cac clang::Sema::ActOnFinishDelayedMemberInitializers(clang::Decl*) + 32
9 clang 0x000000010cd3b943 clang::Parser::ParseLexedMemberInitializers(clang::Parser::ParsingClass&) + 363
10 clang 0x000000010cd514f0 clang::Parser::ParseCXXMemberSpecification(clang::SourceLocation, clang::SourceLocation, clang::Parser::ParsedAttributesWithRange&, unsigned int, clang::Decl*) + 2404
11 clang 0x000000010cd50713 clang::Parser::ParseClassSpecifier(clang::tok::TokenKind, clang::SourceLocation, clang::DeclSpec&, clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier, bool, clang::Parser::DeclSpecContext, clang::Parser::ParsedAttributesWithRange&) + 5367
12 clang 0x000000010cd4277e clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier, clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*) + 1266
13 clang 0x000000010cd808bb clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) + 73
14 clang 0x000000010cd806bf clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) + 233
15 clang 0x000000010cd7ffca clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) + 1818
16 clang 0x000000010cd7f880 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) + 238
17 clang 0x000000010cd39f45 clang::ParseAST(clang::Sema&, bool, bool) + 289
18 clang 0x000000010ccd2284 clang::FrontendAction::Execute() + 106
19 clang 0x000000010ccb6cf0 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 540
20 clang 0x000000010c987f80 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 3340
21 clang 0x000000010c23f18b cc1_main(char const**, char const**, char const*, void*) + 681
22 clang 0x000000010c23e779 main + 6725
23 libdyld.dylib 0x00007fff92d3e7e1 start + 0
24 libdyld.dylib 0x0000000000000004 start + 1831606307
Stack dump:
0. Program arguments: ../../build-upstream-ninja/bin/clang -cc1 -std=c++11 exception-spec-crash.cpp
1. exception-spec-crash.cpp:25:2: current parser token ';'
2. exception-spec-crash.cpp:21:1: parsing struct/union/class body 'Foo'
More information about the cfe-commits
mailing list