Fixed possible crash in NotifyWindow click handler.

Adding information about crashed string in TextBlock parser.

Telegram/SourceFiles/app.cpp

@@ -447,7 +447,7 @@ namespace App {
 					QString pname = (showPhoneChanged || phoneChanged || nameChanged) ? ((showPhone && !phone.isEmpty()) ? formatPhone(phone) : QString()) : data->nameOrPhone;
 
 					if (!minimal && d.is_self() && uname != data->username) {
-						SignalHandlers::setSelfUsername(uname);
+						SignalHandlers::setCrashAnnotation("Username", uname);
 					}
 					data->setName(fname, lname, pname, uname);
 					if (d.has_photo()) {

Telegram/SourceFiles/gui/text.cpp

@@ -3533,7 +3533,14 @@ TextBlock::TextBlock(const style::font &font, const QString &str, QFixed minResi
 		layout.beginLayout();
 		layout.createLine();
 
+		bool logCrashString = (rand_value<uchar>() % 4 == 1);
+		if (logCrashString) {
+			SignalHandlers::setCrashAnnotationRef("CrashString", &part);
+		}
 		BlockParser parser(&engine, this, minResizeWidth, _from, part);
+		if (logCrashString) {
+			SignalHandlers::clearCrashAnnotationRef("CrashString");
+		}
 
 		layout.endLayout();
 	}

Telegram/SourceFiles/logs.cpp

@@ -614,119 +614,127 @@ void _moveOldDataFiles(const QString &wasDir) {
 
 namespace SignalHandlers {
 
-	typedef std::map<std::string, std::string> AnnotationsMap;
+namespace internal {
-	AnnotationsMap ProcessAnnotations;
+	using Annotations = std::map<std::string, std::string>;
+	using AnnotationRefs = std::map<std::string, const QString*>;
+
+	Annotations ProcessAnnotations;
+	AnnotationRefs ProcessAnnotationRefs;
 
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
 
-	QString CrashDumpPath;
+	QString ReportPath;
-	FILE *CrashDumpFile = nullptr;
+	FILE *ReportFile = nullptr;
-	int CrashDumpFileNo = 0;
+	int ReportFileNo = 0;
 	char LaunchedDateTimeStr[32] = { 0 };
 	char LaunchedBinaryName[256] = { 0 };
 
-	void _writeChar(char ch) {
+	void writeChar(char ch) {
-		fwrite(&ch, 1, 1, CrashDumpFile);
+		fwrite(&ch, 1, 1, ReportFile);
 	}
 
-	dump::~dump() {
-		if (CrashDumpFile) {
-			fflush(CrashDumpFile);
-		}
-	}
-
-	const dump &operator<<(const dump &stream, const char *str) {
-		if (!CrashDumpFile) return stream;
-
-		fwrite(str, 1, strlen(str), CrashDumpFile);
-		return stream;
-	}
-
-    const dump &operator<<(const dump &stream, const wchar_t *str) {
-        if (!CrashDumpFile) return stream;
-
-        for (int i = 0, l = wcslen(str); i < l; ++i) {
-            if (str[i] >= 0 && str[i] < 128) {
-                _writeChar(char(str[i]));
-            } else {
-                _writeChar('?');
-            }
-        }
-        return stream;
-    }
-
 	template <bool Unsigned, typename Type>
-	struct _writeNumberSignAndRemoveIt {
+	struct writeNumberSignAndRemoveIt {
 		static void call(Type &number) {
 			if (number < 0) {
-				_writeChar('-');
+				writeChar('-');
 				number = -number;
 			}
 		}
 	};
 	template <typename Type>
-	struct _writeNumberSignAndRemoveIt<true, Type> {
+	struct writeNumberSignAndRemoveIt<true, Type> {
 		static void call(Type &number) {
 		}
 	};
 
 	template <typename Type>
-	const dump &_writeNumber(const dump &stream, Type number) {
+	const dump &writeNumber(const dump &stream, Type number) {
-		if (!CrashDumpFile) return stream;
+		if (!ReportFile) return stream;
 
-		_writeNumberSignAndRemoveIt<(Type(-1) > Type(0)), Type>::call(number);
+		writeNumberSignAndRemoveIt<(Type(-1) > Type(0)), Type>::call(number);
 		Type upper = 1, prev = number / 10;
 		while (prev >= upper) {
 			upper *= 10;
 		}
 		while (upper > 0) {
 			int digit = (number / upper);
-			_writeChar('0' + digit);
+			internal::writeChar('0' + digit);
 			number -= digit * upper;
 			upper /= 10;
 		}
 		return stream;
 	}
 
+} // namespace internal
+
+	dump::~dump() {
+		if (internal::ReportFile) {
+			fflush(internal::ReportFile);
+		}
+	}
+
+	const dump &operator<<(const dump &stream, const char *str) {
+		if (!internal::ReportFile) return stream;
+
+		fwrite(str, 1, strlen(str), internal::ReportFile);
+		return stream;
+	}
+
+    const dump &operator<<(const dump &stream, const wchar_t *str) {
+        if (!internal::ReportFile) return stream;
+
+        for (int i = 0, l = wcslen(str); i < l; ++i) {
+            if (str[i] >= 0 && str[i] < 128) {
+				internal::writeChar(char(str[i]));
+            } else {
+				internal::writeChar('?');
+            }
+        }
+        return stream;
+    }
+
 	const dump &operator<<(const dump &stream, int num) {
-		return _writeNumber(stream, num);
+		return internal::writeNumber(stream, num);
 	}
 
 	const dump &operator<<(const dump &stream, unsigned int num) {
-		return _writeNumber(stream, num);
+		return internal::writeNumber(stream, num);
 	}
 
 	const dump &operator<<(const dump &stream, unsigned long num) {
-		return _writeNumber(stream, num);
+		return internal::writeNumber(stream, num);
 	}
 
 	const dump &operator<<(const dump &stream, unsigned long long num) {
-		return _writeNumber(stream, num);
+		return internal::writeNumber(stream, num);
 	}
 
 	const dump &operator<<(const dump &stream, double num) {
 		if (num < 0) {
-			_writeChar('-');
+			internal::writeChar('-');
 			num = -num;
 		}
-		_writeNumber(stream, uint64(floor(num)));
+		internal::writeNumber(stream, uint64(floor(num)));
-		_writeChar('.');
+		internal::writeChar('.');
 		num -= floor(num);
 		for (int i = 0; i < 4; ++i) {
 			num *= 10;
 			int digit = int(floor(num));
-			_writeChar('0' + digit);
+			internal::writeChar('0' + digit);
 			num -= digit;
 		}
 		return stream;
 	}
 
-	Qt::HANDLE LoggingCrashThreadId = 0;
+namespace internal {
-	bool LoggingCrashHeaderWritten = false;
-	QMutex LoggingCrashMutex;
 
-	const char *BreakpadDumpPath = 0;
+	Qt::HANDLE ReportingThreadId = nullptr;
-    const wchar_t *BreakpadDumpPathW = 0;
+	bool ReportingHeaderWritten = false;
+	QMutex ReportingMutex;
+
+	const char *BreakpadDumpPath = nullptr;
+	const wchar_t *BreakpadDumpPathW = nullptr;
 
 #if defined Q_OS_MAC || defined Q_OS_LINUX32 || defined Q_OS_LINUX64
 	struct sigaction SIG_def[32];
@@ -753,14 +761,19 @@ namespace SignalHandlers {
 		}
 
 		Qt::HANDLE thread = QThread::currentThreadId();
-		if (thread == LoggingCrashThreadId) return;
+		if (thread == ReportingThreadId) return;
 
-		QMutexLocker lock(&LoggingCrashMutex);
+		QMutexLocker lock(&ReportingMutex);
-		LoggingCrashThreadId = thread;
+		ReportingThreadId = thread;
 
-		if (!LoggingCrashHeaderWritten) {
+		if (!ReportingHeaderWritten) {
-			LoggingCrashHeaderWritten = true;
+			ReportingHeaderWritten = true;
-			const AnnotationsMap c_ProcessAnnotations(ProcessAnnotations);
+
+			for (const auto &i : ProcessAnnotationRefs) {
+				ProcessAnnotations[i.first] = i.second->toUtf8().constData();
+			}
+
+			const Annotations c_ProcessAnnotations(ProcessAnnotations);
 			for (const auto &i : c_ProcessAnnotations) {
 				dump() << i.first.c_str() << ": " << i.second.c_str() << "\n";
 			}
@@ -843,7 +856,7 @@ namespace SignalHandlers {
 
 		dump() << "\nBacktrace:\n";
 
-		backtrace_symbols_fd(addresses, size, CrashDumpFileNo);
+		backtrace_symbols_fd(addresses, size, ReportFileNo);
 
 #else // Q_OS_MAC || Q_OS_LINUX32 || Q_OS_LINUX64
 		dump() << "\nBacktrace:\n";
@@ -853,7 +866,7 @@ namespace SignalHandlers {
 
 		dump() << "\n";
 
-		LoggingCrashThreadId = 0;
+		ReportingThreadId = nullptr;
 	}
 
 	bool SetSignalHandlers = true;
@@ -890,8 +903,12 @@ namespace SignalHandlers {
 
 #endif // !TDESKTOP_DISABLE_CRASH_REPORTS
 
+} // namespace internal
+
 	void StartCrashHandler() {
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
+		using internal::ProcessAnnotations;
+
 		ProcessAnnotations["Binary"] = cExeName().toUtf8().constData();
 		ProcessAnnotations["ApiId"] = QString::number(ApiId).toUtf8().constData();
 		ProcessAnnotations["Version"] = (cBetaVersion() ? qsl("%1 beta").arg(cBetaVersion()) : (cDevVersion() ? qsl("%1 dev") : qsl("%1")).arg(AppVersion)).toUtf8().constData();
@@ -903,10 +920,10 @@ namespace SignalHandlers {
 		QDir().mkpath(dumpspath);
 
 #ifdef Q_OS_WIN
-		BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
+		internal::BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
 			dumpspath.toStdWString(),
 			/*FilterCallback*/ 0,
-			DumpCallback,
+			internal::DumpCallback,
 			/*context*/	0,
 			true
 		);
@@ -914,16 +931,16 @@ namespace SignalHandlers {
 
 #ifdef MAC_USE_BREAKPAD
 #ifndef _DEBUG
-		BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
+		internal::BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
 			QFile::encodeName(dumpspath).toStdString(),
 			/*FilterCallback*/ 0,
-			DumpCallback,
+			internal::DumpCallback,
 			/*context*/ 0,
 			true,
 			0
 		);
 #endif // !_DEBUG
-		SetSignalHandlers = false;
+		internal::SetSignalHandlers = false;
 #else // MAC_USE_BREAKPAD
 		crashpad::CrashpadClient crashpad_client;
 		std::string handler = (cExeDir() + cExeName() + qsl("/Contents/Helpers/crashpad_handler")).toUtf8().constData();
@@ -938,10 +955,10 @@ namespace SignalHandlers {
 		}
 #endif // else for MAC_USE_BREAKPAD
 #elif defined Q_OS_LINUX64 || defined Q_OS_LINUX32
-		BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
+		internal::BreakpadExceptionHandler = new google_breakpad::ExceptionHandler(
 			google_breakpad::MinidumpDescriptor(QFile::encodeName(dumpspath).toStdString()),
 			/*FilterCallback*/ 0,
-			DumpCallback,
+			internal::DumpCallback,
 			/*context*/ 0,
 			true,
 			-1
@@ -954,9 +971,8 @@ namespace SignalHandlers {
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
 
 #if !defined Q_OS_MAC || defined MAC_USE_BREAKPAD
-		if (BreakpadExceptionHandler) {
+		if (internal::BreakpadExceptionHandler) {
-			google_breakpad::ExceptionHandler *h = BreakpadExceptionHandler;
+			google_breakpad::ExceptionHandler *h = getPointerAndReset(internal::BreakpadExceptionHandler);
-			BreakpadExceptionHandler = 0;
 			delete h;
 		}
 #endif // !Q_OS_MAC || MAC_USE_BREAKPAD
@@ -966,15 +982,16 @@ namespace SignalHandlers {
 
 	Status start() {
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
-		CrashDumpPath = cWorkingDir() + qsl("tdata/working");
+		using internal::ReportPath;
+		ReportPath = cWorkingDir() + qsl("tdata/working");
 
 #ifdef Q_OS_WIN
 		FILE *f = nullptr;
-		if (_wfopen_s(&f, CrashDumpPath.toStdWString().c_str(), L"rb") != 0) {
+		if (_wfopen_s(&f, ReportPath.toStdWString().c_str(), L"rb") != 0) {
 			f = nullptr;
 		} else {
 #else // !Q_OS_WIN
-		if (FILE *f = fopen(QFile::encodeName(CrashDumpPath).constData(), "rb")) {
+		if (FILE *f = fopen(QFile::encodeName(ReportPath).constData(), "rb")) {
 #endif // else for !Q_OS_WIN
 			QByteArray lastdump;
 			char buffer[256 * 1024] = { 0 };
@@ -986,7 +1003,7 @@ namespace SignalHandlers {
 
 			Sandbox::SetLastCrashDump(lastdump);
 
-			LOG(("Opened '%1' for reading, the previous Telegram Desktop launch was not finished properly :( Crash log size: %2").arg(CrashDumpPath).arg(lastdump.size()));
+			LOG(("Opened '%1' for reading, the previous Telegram Desktop launch was not finished properly :( Crash log size: %2").arg(ReportPath).arg(lastdump.size()));
 
 			return LastCrashed;
 		}
@@ -997,28 +1014,28 @@ namespace SignalHandlers {
 
 	Status restart() {
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
-		if (CrashDumpFile) {
+		if (internal::ReportFile) {
 			return Started;
 		}
 
 #ifdef Q_OS_WIN
-		if (_wfopen_s(&CrashDumpFile, CrashDumpPath.toStdWString().c_str(), L"wb") != 0) {
+		if (_wfopen_s(&internal::ReportFile, internal::ReportPath.toStdWString().c_str(), L"wb") != 0) {
-			CrashDumpFile = nullptr;
+			internal::ReportFile = nullptr;
 		}
 #else // Q_OS_WIN
 		CrashDumpFile = fopen(QFile::encodeName(CrashDumpPath).constData(), "wb");
 #endif // else for Q_OS_WIN
-		if (CrashDumpFile) {
+		if (internal::ReportFile) {
 #ifdef Q_OS_WIN
-			CrashDumpFileNo = _fileno(CrashDumpFile);
+			internal::ReportFileNo = _fileno(internal::ReportFile);
 #else // Q_OS_WIN
-			CrashDumpFileNo = fileno(CrashDumpFile);
+			CrashDumpFileNo = fileno(internal::ReportFile);
 #endif // else for Q_OS_WIN
-			if (SetSignalHandlers) {
+			if (internal::SetSignalHandlers) {
 #ifndef Q_OS_WIN
 				struct sigaction sigact;
 
-				sigact.sa_sigaction = SignalHandlers::Handler;
+				sigact.sa_sigaction = SignalHandlers::internal::Handler;
 				sigemptyset(&sigact.sa_mask);
 				sigact.sa_flags = SA_NODEFER | SA_RESETHAND | SA_SIGINFO;
 
@@ -1029,16 +1046,16 @@ namespace SignalHandlers {
 				sigaction(SIGBUS, &sigact, &SIG_def[SIGBUS]);
 				sigaction(SIGSYS, &sigact, &SIG_def[SIGSYS]);
 #else // !Q_OS_WIN
-				signal(SIGABRT, SignalHandlers::Handler);
+				signal(SIGABRT, SignalHandlers::internal::Handler);
-				signal(SIGSEGV, SignalHandlers::Handler);
+				signal(SIGSEGV, SignalHandlers::internal::Handler);
-				signal(SIGILL, SignalHandlers::Handler);
+				signal(SIGILL, SignalHandlers::internal::Handler);
-				signal(SIGFPE, SignalHandlers::Handler);
+				signal(SIGFPE, SignalHandlers::internal::Handler);
 #endif // else for !Q_OS_WIN
 			}
 			return Started;
 		}
 
-		LOG(("FATAL: Could not open '%1' for writing!").arg(CrashDumpPath));
+		LOG(("FATAL: Could not open '%1' for writing!").arg(internal::ReportPath));
 
 		return CantOpen;
 #else // !TDESKTOP_DISABLE_CRASH_REPORTS
@@ -1049,29 +1066,33 @@ namespace SignalHandlers {
 	void finish() {
 #ifndef TDESKTOP_DISABLE_CRASH_REPORTS
 		FinishCrashHandler();
-		if (CrashDumpFile) {
+		if (internal::ReportFile) {
-			fclose(CrashDumpFile);
+			fclose(internal::ReportFile);
-			CrashDumpFile = nullptr;
+			internal::ReportFile = nullptr;
 
 #ifdef Q_OS_WIN
-			_wunlink(CrashDumpPath.toStdWString().c_str());
+			_wunlink(internal::ReportPath.toStdWString().c_str());
 #else // Q_OS_WIN
-			unlink(CrashDumpPath.toUtf8().constData());
+			unlink(internal::ReportPath.toUtf8().constData());
 #endif // else for Q_OS_WIN
 		}
 #endif // !TDESKTOP_DISABLE_CRASH_REPORTS
 	}
 
-	void setSelfUsername(const QString &username) {
+	void setCrashAnnotation(const std::string &key, const QString &value) {
-		if (username.trimmed().isEmpty()) {
+		if (value.trimmed().isEmpty()) {
-			ProcessAnnotations.erase("Username");
+			internal::ProcessAnnotations.erase(key);
 		} else {
-			ProcessAnnotations["Username"] = username.toUtf8().constData();
+			internal::ProcessAnnotations[key] = value.toUtf8().constData();
 		}
 	}
 
-	void setAssertionInfo(const QString &info) {
+	void setCrashAnnotationRef(const std::string &key, const QString *valuePtr) {
-		ProcessAnnotations["Assertion"] = info.toUtf8().constData();
+		if (valuePtr) {
+			internal::ProcessAnnotationRefs.erase(key);
+		} else {
+			internal::ProcessAnnotationRefs[key] = valuePtr;
+		}
 	}
 
 }

Telegram/SourceFiles/logs.h

@@ -107,7 +107,13 @@ namespace SignalHandlers {
 	Status restart(); // can be only CantOpen or Started
 	void finish();
 
-	void setSelfUsername(const QString &username);
+	void setCrashAnnotation(const std::string &key, const QString &value);
-	void setAssertionInfo(const QString &info);
+
+	// Remembers value pointer and tries to add the value to the crash report.
+	// Attention! You should call clearCrashAnnotationRef(key) before destroying value.
+	void setCrashAnnotationRef(const std::string &key, const QString *valuePtr);
+	inline void clearCrashAnnotationRef(const std::string &key) {
+		setCrashAnnotationRef(key, nullptr);
+	}
 
 }

Telegram/SourceFiles/types.h

@@ -303,7 +303,7 @@ inline void t_noop() {}
 inline void t_assert_fail(const char *message, const char *file, int32 line) {
 	QString info(qsl("%1 %2:%3").arg(message).arg(file).arg(line));
 	LOG(("Assertion Failed! %1 %2:%3").arg(info));
-	SignalHandlers::setAssertionInfo(info);
+	SignalHandlers::setCrashAnnotation("Assertion", info);
 	*t_assert_nullptr = 0;
 }
 #define t_assert_full(condition, message, file, line) ((!(condition)) ? t_assert_fail(message, file, line) : t_noop())

Telegram/SourceFiles/window.cpp

@@ -287,18 +287,20 @@ void NotifyWindow::startHiding() {
 
 void NotifyWindow::mousePressEvent(QMouseEvent *e) {
 	if (!history) return;
+
 	PeerId peer = history->peer->id;
+	MsgId msgId = (!history->peer->isUser() && item && item->mentionsMe() && item->id > 0) ? item->id : ShowAtUnreadMsgId;
 
 	if (e->button() == Qt::RightButton) {
 		unlinkHistoryAndNotify();
-	} else if (history) {
+	} else {
 		App::wnd()->showFromTray();
 		if (App::passcoded()) {
 			App::wnd()->setInnerFocus();
 			App::wnd()->notifyClear();
 		} else {
 			App::wnd()->hideSettings();
-			Ui::showPeerHistory(peer, (!history->peer->isUser() && item && item->mentionsMe() && item->id > 0) ? item->id : ShowAtUnreadMsgId);
+			Ui::showPeerHistory(peer, msgId);
 		}
 		e->ignore();
 	}