From fe0a091802ee8840fc8026befe19b27f91eef1e8 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 6 Jan 2017 16:50:37 +0100 Subject: [PATCH] C++: Fix use-after-free crash when handling auto expressions The Control of the Document "exprDoc" in ResolveExpression::visit( SimpleNameAST*ast) owns names that are passed on further as part of the LookupItems. However, the life time of that Document and thus the Control ends in that function. Fix by using the appropriate Control object. Task-number: QTCREATORBUG-16731 Change-Id: I5a7af0a67613fff79f7e07865801585c13bb9b45 Reviewed-by: Orgad Shaneh --- src/libs/cplusplus/CppDocument.cpp | 28 ++++++++++++++++++--- src/libs/cplusplus/CppDocument.h | 1 + src/libs/cplusplus/ResolveExpression.cpp | 31 +++++++++++++++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/libs/cplusplus/CppDocument.cpp b/src/libs/cplusplus/CppDocument.cpp index 80476dd8b1e..93f3654e4fb 100644 --- a/src/libs/cplusplus/CppDocument.cpp +++ b/src/libs/cplusplus/CppDocument.cpp @@ -286,8 +286,10 @@ Document::~Document() { delete _translationUnit; _translationUnit = 0; - delete _control->diagnosticClient(); - delete _control; + if (_control) { + delete _control->diagnosticClient(); + delete _control; + } _control = 0; } @@ -296,6 +298,25 @@ Control *Document::control() const return _control; } +Control *Document::swapControl(Control *newControl) +{ + if (newControl) { + const StringLiteral *fileId = newControl->stringLiteral(_translationUnit->fileId()->chars(), + _translationUnit->fileId()->size()); + const auto newTranslationUnit = new TranslationUnit(newControl, fileId); + newTranslationUnit->setLanguageFeatures(_translationUnit->languageFeatures()); + delete _translationUnit; + _translationUnit = newTranslationUnit; + } else { + delete _translationUnit; + _translationUnit = 0; + } + + Control *oldControl = _control; + _control = newControl; + return oldControl; +} + unsigned Document::revision() const { return _revision; @@ -696,7 +717,8 @@ void Document::releaseSourceAndAST() if (!_keepSourceAndASTCount.deref()) { _source.clear(); _translationUnit->release(); - _control->squeeze(); + if (_control) + _control->squeeze(); } } diff --git a/src/libs/cplusplus/CppDocument.h b/src/libs/cplusplus/CppDocument.h index bac2fb2a472..006f17aba68 100644 --- a/src/libs/cplusplus/CppDocument.h +++ b/src/libs/cplusplus/CppDocument.h @@ -78,6 +78,7 @@ public: unsigned bytesOffset, unsigned utf16charsOffset); Control *control() const; + Control *swapControl(Control *newControl); TranslationUnit *translationUnit() const; bool skipFunctionBody() const; diff --git a/src/libs/cplusplus/ResolveExpression.cpp b/src/libs/cplusplus/ResolveExpression.cpp index 2f55c1564f2..69ce4f4cb4e 100644 --- a/src/libs/cplusplus/ResolveExpression.cpp +++ b/src/libs/cplusplus/ResolveExpression.cpp @@ -688,6 +688,31 @@ public: bool _block; }; +class ExpressionDocumentHelper +{ +public: + // Set up an expression document with an external Control + ExpressionDocumentHelper(const QByteArray &utf8code, Control *control) + : document(Document::create(QLatin1String(""))) + { + Control *oldControl = document->swapControl(control); + delete oldControl->diagnosticClient(); + delete oldControl; + document->setUtf8Source(utf8code); + document->parse(Document::ParseExpression); + document->check(); + } + + // Ensure that the external Control is not deleted + ~ExpressionDocumentHelper() + { + document->swapControl(nullptr); + } + +public: + Document::Ptr document; +}; + } // namespace anonymous bool ResolveExpression::visit(SimpleNameAST *ast) @@ -730,9 +755,9 @@ bool ResolveExpression::visit(SimpleNameAST *ast) exprTyper.init(doc, _context.snapshot(), _context.bindings(), QSet(_autoDeclarationsBeingResolved) << decl); - Document::Ptr exprDoc = - documentForExpression(exprTyper.preprocessedExpression(initializer)); - exprDoc->check(); + const ExpressionDocumentHelper exprHelper(exprTyper.preprocessedExpression(initializer), + _context.bindings()->control().data()); + const Document::Ptr exprDoc = exprHelper.document; DeduceAutoCheck deduceAuto(ast->name->identifier(), exprDoc->translationUnit()); if (deduceAuto._block)