Skip to content

[2 patches] Incorrect handling of QT_FATAL_CRITICALS/QT_FATAL_WARNINGS (non-UB data race)

Andreas Sturmlechner requested to merge asturmlechner/qtbase:asturmlechner/QTBUG-115062-data-race into kde/5.15

Still quite fresh, so might want to give it a couple of days still: https://bugreports.qt.io/browse/QTBUG-115062

QLogging: DRY isFatal(QtMsgType)

Extract Method on the count-down algorithm of fatalCriticals and
fatalWarnings, so we don't have the repeat the calculation and the
comment.

Task-number: QTBUG-115062
Pick-to: 6.6 6.5 6.2 5.15
Change-Id: I4bcbc2f5a21b999e7f301085581677b437a889e9
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Kai Köhne <kai.koehne@qt.io>
(cherry picked from commit 3ffc1f9775cde1369ebdfcfa91ec18c393c24260)

Make sure we don't count down past 0 QT_FATAL_CRITICALS

The old code first checked for == 0, then, if false, executed a
fetchAndAdd(-1), both with relaxed memory ordering. This can lead to
executions that, counter to what the code comment states, can count
down past 0:

	// T1                   T2
	loadRelaxed()                                  // true
							loadRelaxed()          // true
	fetchAndAddRelaxed(-1)                         // e.g. 1 → 0
							fetchAndAddRelaxed(-1) // 0 → -1

while fatality is detected exactly once, this execution doesn't stop
at 0 and causes further calls to isFatal() to count down further, with
the (very) remote spectre of underflow past INT_MIN.

Fix by using a CAS loop instead, so each count-down uses only one
step, not two, which therefore can no longer interleave.

Fixes: QTBUG-115062
Pick-to: 6.6 6.5 6.2 5.15
Change-Id: If77b906c94cb4b9fa91bfad84fe63bc8d9103b0a
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit b933a5668cc5647d26378f8a9a52901d0497585d)

Merge request reports