From 5121c009f6163ff1f6d83e4416bb49e0abe7eaf8 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 5 Jul 2019 11:26:49 +0200 Subject: Clang: Fix use-after-free ...introduced with commit d52ac9a7084df220a5349468703c11cc1c0794c4 Clang: Fix unresolved #includes for ui_*.h headers ClangCodeModelServer::onJobFinished might take over ownership of the jobs object. This should be reflected in the JobFinishedCallback as otherwise we will continue in Jobs::onJobFinished() right into a use- after-free. Change-Id: I4b5cbdf781a8e66f45a743c451b4484bf922f720 Reviewed-by: Marco Bubke Reviewed-by: Cristian Adam --- src/tools/clangbackend/source/clangcodemodelserver.cpp | 12 ++++++++---- src/tools/clangbackend/source/clangcodemodelserver.h | 4 ++-- src/tools/clangbackend/source/clangjobs.cpp | 3 ++- src/tools/clangbackend/source/clangjobs.h | 2 +- .../source/clangsupportivetranslationunitinitializer.cpp | 1 + 5 files changed, 14 insertions(+), 8 deletions(-) (limited to 'src/tools') diff --git a/src/tools/clangbackend/source/clangcodemodelserver.cpp b/src/tools/clangbackend/source/clangcodemodelserver.cpp index e82766cf5e..d521fee1c8 100644 --- a/src/tools/clangbackend/source/clangcodemodelserver.cpp +++ b/src/tools/clangbackend/source/clangcodemodelserver.cpp @@ -103,7 +103,7 @@ void ClangCodeModelServer::documentsOpened(const ClangBackEnd::DocumentsOpenedMe document.setDirtyIfDependencyIsMet(document.filePath()); DocumentProcessor processor = documentProcessors().create(document); processor.jobs().setJobFinishedCallback([this](const Jobs::RunningJob &a, IAsyncJob *b) { - onJobFinished(a, b); + return onJobFinished(a, b); }); } const std::vector resetDocuments_ = resetDocuments(toReset); @@ -401,12 +401,14 @@ void ClangCodeModelServer::processSuspendResumeJobs(const std::vector } } -void ClangCodeModelServer::onJobFinished(const Jobs::RunningJob &jobRecord, IAsyncJob *job) +bool ClangCodeModelServer::onJobFinished(const Jobs::RunningJob &jobRecord, IAsyncJob *job) { if (jobRecord.jobRequest.type == JobRequest::Type::UpdateAnnotations) { const auto updateJob = static_cast(job); - resetDocumentsWithUnresolvedIncludes({updateJob->pinnedDocument()}); + return resetDocumentsWithUnresolvedIncludes({updateJob->pinnedDocument()}); } + + return false; } void ClangCodeModelServer::categorizeFileContainers(const QVector &fileContainers, @@ -476,7 +478,7 @@ static bool isDocumentWithUnresolvedIncludesFixable(const Document &document, return false; } -void ClangCodeModelServer::resetDocumentsWithUnresolvedIncludes( +int ClangCodeModelServer::resetDocumentsWithUnresolvedIncludes( const std::vector &documents) { DocumentResetInfos toReset; @@ -489,6 +491,8 @@ void ClangCodeModelServer::resetDocumentsWithUnresolvedIncludes( } resetDocuments(toReset); + + return toReset.size(); } void ClangCodeModelServer::setUpdateAnnotationsTimeOutInMsForTestsOnly(int value) diff --git a/src/tools/clangbackend/source/clangcodemodelserver.h b/src/tools/clangbackend/source/clangcodemodelserver.h index 14fefadcf2..471c27bd8d 100644 --- a/src/tools/clangbackend/source/clangcodemodelserver.h +++ b/src/tools/clangbackend/source/clangcodemodelserver.h @@ -83,13 +83,13 @@ private: void processTimerForVisibleButNotCurrentDocuments(); void processSuspendResumeJobs(const std::vector &documents); - void onJobFinished(const Jobs::RunningJob &jobRecord, IAsyncJob *job); + bool onJobFinished(const Jobs::RunningJob &jobRecord, IAsyncJob *job); void categorizeFileContainers(const QVector &fileContainers, QVector &toCreate, DocumentResetInfos &toReset) const; std::vector resetDocuments(const DocumentResetInfos &infos); - void resetDocumentsWithUnresolvedIncludes(const std::vector &documents); + int resetDocumentsWithUnresolvedIncludes(const std::vector &documents); void addAndRunUpdateJobs(std::vector documents); diff --git a/src/tools/clangbackend/source/clangjobs.cpp b/src/tools/clangbackend/source/clangjobs.cpp index 572bb9bde3..f1df7dcd49 100644 --- a/src/tools/clangbackend/source/clangjobs.cpp +++ b/src/tools/clangbackend/source/clangjobs.cpp @@ -170,7 +170,8 @@ void Jobs::onJobFinished(IAsyncJob *asyncJob) if (m_jobFinishedCallback) { const RunningJob runningJob = m_running.value(asyncJob); - m_jobFinishedCallback(runningJob, asyncJob); + if (m_jobFinishedCallback(runningJob, asyncJob)) + return; } m_running.remove(asyncJob); diff --git a/src/tools/clangbackend/source/clangjobs.h b/src/tools/clangbackend/source/clangjobs.h index 3b4966a8e1..3d8af16fe4 100644 --- a/src/tools/clangbackend/source/clangjobs.h +++ b/src/tools/clangbackend/source/clangjobs.h @@ -50,7 +50,7 @@ public: }; using RunningJobs = QHash; - using JobFinishedCallback = std::function; + using JobFinishedCallback = std::function; public: Jobs(Documents &documents, diff --git a/src/tools/clangbackend/source/clangsupportivetranslationunitinitializer.cpp b/src/tools/clangbackend/source/clangsupportivetranslationunitinitializer.cpp index 07d50a1411..a34b7bb114 100644 --- a/src/tools/clangbackend/source/clangsupportivetranslationunitinitializer.cpp +++ b/src/tools/clangbackend/source/clangsupportivetranslationunitinitializer.cpp @@ -60,6 +60,7 @@ void SupportiveTranslationUnitInitializer::startInitializing() m_jobs.setJobFinishedCallback([this](const Jobs::RunningJob &runningJob, IAsyncJob *) { checkIfParseJobFinished(runningJob); + return false; }); addJob(JobRequest::Type::ParseSupportiveTranslationUnit); m_jobs.process(); -- cgit v1.2.3