From a23470f4b8cb63e63462e8e8b0f2c1208faf4db1 Mon Sep 17 00:00:00 2001 From: John Preston Date: Thu, 31 Mar 2016 15:55:25 +0400 Subject: [PATCH] Fixed possible crash in NotifyWindow click handler. Adding information about crashed string in TextBlock parser. --- Telegram/SourceFiles/app.cpp | 2 +- Telegram/SourceFiles/gui/text.cpp | 7 + Telegram/SourceFiles/logs.cpp | 219 ++++++++++++++++-------------- Telegram/SourceFiles/logs.h | 10 +- Telegram/SourceFiles/types.h | 2 +- Telegram/SourceFiles/window.cpp | 6 +- 6 files changed, 141 insertions(+), 105 deletions(-) diff --git a/Telegram/SourceFiles/app.cpp b/Telegram/SourceFiles/app.cpp index 731a57ee1..18dfe7ae6 100644 --- a/Telegram/SourceFiles/app.cpp +++ b/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()) { diff --git a/Telegram/SourceFiles/gui/text.cpp b/Telegram/SourceFiles/gui/text.cpp index f350a1f3e..b12fdfe3f 100644 --- a/Telegram/SourceFiles/gui/text.cpp +++ b/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() % 4 == 1); + if (logCrashString) { + SignalHandlers::setCrashAnnotationRef("CrashString", &part); + } BlockParser parser(&engine, this, minResizeWidth, _from, part); + if (logCrashString) { + SignalHandlers::clearCrashAnnotationRef("CrashString"); + } layout.endLayout(); } diff --git a/Telegram/SourceFiles/logs.cpp b/Telegram/SourceFiles/logs.cpp index 68fa217d0..916193635 100644 --- a/Telegram/SourceFiles/logs.cpp +++ b/Telegram/SourceFiles/logs.cpp @@ -614,119 +614,127 @@ void _moveOldDataFiles(const QString &wasDir) { namespace SignalHandlers { - typedef std::map AnnotationsMap; - AnnotationsMap ProcessAnnotations; +namespace internal { + using Annotations = std::map; + using AnnotationRefs = std::map; + + Annotations ProcessAnnotations; + AnnotationRefs ProcessAnnotationRefs; #ifndef TDESKTOP_DISABLE_CRASH_REPORTS - QString CrashDumpPath; - FILE *CrashDumpFile = nullptr; - int CrashDumpFileNo = 0; + QString ReportPath; + FILE *ReportFile = nullptr; + int ReportFileNo = 0; char LaunchedDateTimeStr[32] = { 0 }; char LaunchedBinaryName[256] = { 0 }; - void _writeChar(char ch) { - fwrite(&ch, 1, 1, CrashDumpFile); + void writeChar(char ch) { + 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 - struct _writeNumberSignAndRemoveIt { + struct writeNumberSignAndRemoveIt { static void call(Type &number) { if (number < 0) { - _writeChar('-'); + writeChar('-'); number = -number; } } }; template - struct _writeNumberSignAndRemoveIt { + struct writeNumberSignAndRemoveIt { static void call(Type &number) { } }; template - const dump &_writeNumber(const dump &stream, Type number) { - if (!CrashDumpFile) return stream; + const dump &writeNumber(const dump &stream, Type number) { + 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))); - _writeChar('.'); + internal::writeNumber(stream, uint64(floor(num))); + 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; - bool LoggingCrashHeaderWritten = false; - QMutex LoggingCrashMutex; +namespace internal { - const char *BreakpadDumpPath = 0; - const wchar_t *BreakpadDumpPathW = 0; + Qt::HANDLE ReportingThreadId = nullptr; + 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); - LoggingCrashThreadId = thread; + QMutexLocker lock(&ReportingMutex); + ReportingThreadId = thread; - if (!LoggingCrashHeaderWritten) { - LoggingCrashHeaderWritten = true; - const AnnotationsMap c_ProcessAnnotations(ProcessAnnotations); + if (!ReportingHeaderWritten) { + ReportingHeaderWritten = true; + + 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) { - google_breakpad::ExceptionHandler *h = BreakpadExceptionHandler; - BreakpadExceptionHandler = 0; + if (internal::BreakpadExceptionHandler) { + google_breakpad::ExceptionHandler *h = getPointerAndReset(internal::BreakpadExceptionHandler); 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) { - CrashDumpFile = nullptr; + if (_wfopen_s(&internal::ReportFile, internal::ReportPath.toStdWString().c_str(), L"wb") != 0) { + 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(SIGSEGV, SignalHandlers::Handler); - signal(SIGILL, SignalHandlers::Handler); - signal(SIGFPE, SignalHandlers::Handler); + signal(SIGABRT, SignalHandlers::internal::Handler); + signal(SIGSEGV, SignalHandlers::internal::Handler); + signal(SIGILL, SignalHandlers::internal::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) { - fclose(CrashDumpFile); - CrashDumpFile = nullptr; + if (internal::ReportFile) { + fclose(internal::ReportFile); + 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) { - if (username.trimmed().isEmpty()) { - ProcessAnnotations.erase("Username"); + void setCrashAnnotation(const std::string &key, const QString &value) { + if (value.trimmed().isEmpty()) { + internal::ProcessAnnotations.erase(key); } else { - ProcessAnnotations["Username"] = username.toUtf8().constData(); + internal::ProcessAnnotations[key] = value.toUtf8().constData(); } } - void setAssertionInfo(const QString &info) { - ProcessAnnotations["Assertion"] = info.toUtf8().constData(); + void setCrashAnnotationRef(const std::string &key, const QString *valuePtr) { + if (valuePtr) { + internal::ProcessAnnotationRefs.erase(key); + } else { + internal::ProcessAnnotationRefs[key] = valuePtr; + } } } diff --git a/Telegram/SourceFiles/logs.h b/Telegram/SourceFiles/logs.h index 6e997e4cf..737d6ecff 100644 --- a/Telegram/SourceFiles/logs.h +++ b/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 setAssertionInfo(const QString &info); + void setCrashAnnotation(const std::string &key, const QString &value); + + // 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); + } } diff --git a/Telegram/SourceFiles/types.h b/Telegram/SourceFiles/types.h index 27515b177..3a775b9cb 100644 --- a/Telegram/SourceFiles/types.h +++ b/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()) diff --git a/Telegram/SourceFiles/window.cpp b/Telegram/SourceFiles/window.cpp index d2ce17a4f..6decac3f6 100644 --- a/Telegram/SourceFiles/window.cpp +++ b/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(); }