diff options
author | Nikolai Kosjar <nikolai.kosjar@qt.io> | 2020-05-14 09:28:47 +0200 |
---|---|---|
committer | Nikolai Kosjar <nikolai.kosjar@qt.io> | 2020-05-15 05:09:45 +0000 |
commit | 9edf0056aed137466a2de7a1ebfaf6bd7b4d95db (patch) | |
tree | 8c9d2771df4b7ca72f8268e02bfed0ae188e2e2a | |
parent | c59e8ce65157b8b8a170179f4dd70e4053550623 (diff) |
ClangTools: Fix crash when starting the analyzer again
Address a nullptr dereference of m_runWorker in
ClangTool::updateForCurrentState() for the case described as (C2) below,
reproducible with "./qtcreator -test ClangTools".
Two use cases are connected to this:
(C1) Run the analyzer twice with clearing app output pane in-between
(C2) Run the analyzer twice without clearing app output pane in-between
Relevant observations in this context are:
(O1) Closing the app output pane destroys the RunControl/RunWorker.
(O2) Running the analyzer a second time will first create a new
RunControl/RunWorker, then destroy the old one.
Now, the change
ClangTools: Avoid accessing deleted run worker
This reverts commit d02f5664e53df41ff7156eb46069e339479074d2.
fixed a use-after-free-crash for (C1), but introduced a
nullptr-deref-crash for (C2) as it resets m_runWorker to nullptr on
RunControl destruction, which conflicts with the order mentioned in
(O2).
To fix both use cases, revert the mentioned change and access
m_runWorker only when we know that it exists for sure - right after
signal emission.
Change-Id: I034f0905d635b15c0c6bbe499648b62d5a058c04
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
-rw-r--r-- | src/plugins/clangtools/clangtool.cpp | 27 | ||||
-rw-r--r-- | src/plugins/clangtools/clangtool.h | 3 |
2 files changed, 19 insertions, 11 deletions
diff --git a/src/plugins/clangtools/clangtool.cpp b/src/plugins/clangtools/clangtool.cpp index e08b73bae6..0928e40e6b 100644 --- a/src/plugins/clangtools/clangtool.cpp +++ b/src/plugins/clangtools/clangtool.cpp @@ -696,9 +696,12 @@ void ClangTool::startTool(ClangTool::FileSelection fileSelection, connect(m_runWorker, &ClangToolRunWorker::buildFailed,this, &ClangTool::onBuildFailed); connect(m_runWorker, &ClangToolRunWorker::startFailed, this, &ClangTool::onStartFailed); connect(m_runWorker, &ClangToolRunWorker::started, this, &ClangTool::onStarted); - connect(m_runWorker, &ClangToolRunWorker::runnerFinished, - this, &ClangTool::updateForCurrentState); - connect(m_runControl, &RunControl::destroyed, [this](){ m_runWorker = nullptr; }); + connect(m_runWorker, &ClangToolRunWorker::runnerFinished, this, [this]() { + m_filesCount = m_runWorker->totalFilesToAnalyze(); + m_filesSucceeded = m_runWorker->filesAnalyzed(); + m_filesFailed = m_runWorker->filesNotAnalyzed(); + updateForCurrentState(); + }); // More init and UI update m_diagnosticFilterModel->setProject(project); @@ -857,6 +860,10 @@ void ClangTool::reset() m_state = State::Initial; m_runControl = nullptr; m_runWorker = nullptr; + + m_filesCount = 0; + m_filesSucceeded = 0; + m_filesFailed = 0; } static bool canAnalyzeProject(Project *project) @@ -1039,8 +1046,6 @@ void ClangTool::onRunControlStopped() void ClangTool::update() { updateForInitialState(); - if (!m_runWorker) - return; updateForCurrentState(); } @@ -1162,9 +1167,9 @@ void ClangTool::updateForCurrentState() // Info bar: errors const bool hasErrorText = !m_infoBarWidget->errorText().isEmpty(); - const bool hasErrors = m_runWorker && m_runWorker->filesNotAnalyzed() > 0; + const bool hasErrors = m_filesFailed > 0; if (hasErrors && !hasErrorText) { - const QString text = makeLink( tr("Failed to analyze %1 files.").arg(m_runWorker->filesNotAnalyzed())); + const QString text = makeLink( tr("Failed to analyze %1 files.").arg(m_filesFailed)); m_infoBarWidget->setError(InfoBarWidget::Warning, text, [this]() { showOutputPane(); }); } @@ -1177,12 +1182,12 @@ void ClangTool::updateForCurrentState() break; case State::AnalyzerRunning: showProgressIcon = true; - if (m_runWorker->totalFilesToAnalyze() == 0) { + if (m_filesCount == 0) { infoText = tr("Analyzing..."); // Not yet fully started/initialized } else { infoText = tr("Analyzing... %1 of %2 files processed.") - .arg(m_runWorker->filesAnalyzed() + m_runWorker->filesNotAnalyzed()) - .arg(m_runWorker->totalFilesToAnalyze()); + .arg(m_filesSucceeded + m_filesFailed) + .arg(m_filesCount); } break; case State::PreparationStarted: @@ -1195,7 +1200,7 @@ void ClangTool::updateForCurrentState() infoText = tr("Analysis stopped by user."); break; case State::AnalyzerFinished: - infoText = tr("Finished processing %1 files.").arg(m_runWorker->totalFilesToAnalyze()); + infoText = tr("Finished processing %1 files.").arg(m_filesCount); break; case State::ImportFinished: infoText = tr("Diagnostics imported."); diff --git a/src/plugins/clangtools/clangtool.h b/src/plugins/clangtools/clangtool.h index 3ee72da83c..a32bb50c2c 100644 --- a/src/plugins/clangtools/clangtool.h +++ b/src/plugins/clangtools/clangtool.h @@ -162,6 +162,9 @@ private: QAction *m_stopAction = nullptr; State m_state = State::Initial; + int m_filesCount = 0; + int m_filesSucceeded = 0; + int m_filesFailed = 0; DiagnosticFilterModel *m_diagnosticFilterModel = nullptr; |