From ba680d6e2e3e4f340ab4547ad218f39ed072f3d6 Mon Sep 17 00:00:00 2001 From: Martin Prikryl Date: Fri, 29 Aug 2025 10:14:53 +0000 Subject: [PATCH] Avoid redundantly capturing exception stack trace Jcl captures exception stack trace on its own, so it's redundant to do it again. And mainly, capturing it the second time somehow does not work with Clang Source commit: 92a3533d082138b3e5c3729ac35f5fbf0b09b57f --- source/core/Exceptions.cpp | 7 +-- source/core/Interface.h | 4 +- source/core/Terminal.cpp | 4 +- source/windows/WinInterface.cpp | 92 ++++++++++----------------------- 4 files changed, 34 insertions(+), 73 deletions(-) diff --git a/source/core/Exceptions.cpp b/source/core/Exceptions.cpp index 4196e7520..c475cc7b7 100644 --- a/source/core/Exceptions.cpp +++ b/source/core/Exceptions.cpp @@ -163,7 +163,7 @@ static bool __fastcall ExceptionMessage(Exception * E, bool Count, { Configuration->Usage->Inc(CounterName); UnicodeString ExceptionDebugInfo = - E->ClassName() + L":" + GetExceptionDebugInfo(); + E->ClassName() + L":" + GetExceptionDebugInfo(E); Configuration->Usage->Set(LastInternalExceptionCounter, ExceptionDebugInfo); } @@ -362,10 +362,7 @@ void __fastcall ExtException::AddMoreMessages(Exception* E) FMoreMessages->Insert(0, UnformatMessage(Msg)); } - if (IsInternalException(E)) - { - AppendExceptionStackTraceAndForget(FMoreMessages); - } + AppendExceptionStackTrace(E, FMoreMessages); if (FMoreMessages->Count == 0) { diff --git a/source/core/Interface.h b/source/core/Interface.h index ca89ec2f2..fd3d815e1 100644 --- a/source/core/Interface.h +++ b/source/core/Interface.h @@ -39,9 +39,9 @@ class TOptions; TOptions * __fastcall GetGlobalOptions(); void __fastcall ShowExtendedException(Exception * E); -bool __fastcall AppendExceptionStackTraceAndForget(TStrings *& MoreMessages); +bool AppendExceptionStackTrace(Exception * E, TStrings *& MoreMessages); void __fastcall IgnoreException(const std::type_info & ExceptionType); -UnicodeString __fastcall GetExceptionDebugInfo(); +UnicodeString GetExceptionDebugInfo(Exception * E); UnicodeString __fastcall GetCompanyRegistryKey(); UnicodeString __fastcall GetRegistryKey(); diff --git a/source/core/Terminal.cpp b/source/core/Terminal.cpp index 751dc2df6..10d1c2d32 100644 --- a/source/core/Terminal.cpp +++ b/source/core/Terminal.cpp @@ -2146,9 +2146,9 @@ unsigned int __fastcall TTerminal::QueryUserException(const UnicodeString Query, } // We know MoreMessages not to be NULL here, - // AppendExceptionStackTraceAndForget should never return true + // AppendExceptionStackTrace should never return true // (indicating it had to create the string list) - DebugAlwaysFalse(AppendExceptionStackTraceAndForget(MoreMessages)); + DebugAlwaysFalse(AppendExceptionStackTrace(E, MoreMessages)); TQueryParams HelpKeywordOverrideParams; if (Params != NULL) diff --git a/source/windows/WinInterface.cpp b/source/windows/WinInterface.cpp index 37d415c5f..edd68a5c4 100644 --- a/source/windows/WinInterface.cpp +++ b/source/windows/WinInterface.cpp @@ -508,11 +508,8 @@ unsigned int __fastcall SimpleErrorDialog(const UnicodeString Msg, const Unicode return Result; } //--------------------------------------------------------------------------- -static TStrings * __fastcall StackInfoListToStrings( - TJclStackInfoList * StackInfoList) +static void FormatStackTrace(TStrings * StackTrace) { - std::unique_ptr StackTrace(new TStringList()); - StackInfoList->AddToStrings(StackTrace.get(), true, false, true, true); for (int Index = 0; Index < StackTrace->Count; Index++) { UnicodeString Frame = StackTrace->Strings[Index]; @@ -531,21 +528,31 @@ static TStrings * __fastcall StackInfoListToStrings( } StackTrace->Strings[Index] = Frame; } +} +//--------------------------------------------------------------------------- +static TStrings * __fastcall StackInfoListToStrings( + TJclStackInfoList * StackInfoList) +{ + std::unique_ptr StackTrace(new TStringList()); + // Corresponds to our JclExceptionStacktraceOptions + StackInfoList->AddToStrings(StackTrace.get(), true, false, true, true); + FormatStackTrace(StackTrace.get()); return StackTrace.release(); } //--------------------------------------------------------------------------- -static std::unique_ptr StackTraceCriticalSection(TraceInitPtr(new TCriticalSection())); -typedef std::map TStackTraceMap; -static TStackTraceMap StackTraceMap; +static TStrings * GetExceptionStackTraceStrings(Exception * E) +{ + std::unique_ptr StackTrace(TextToStringList(E->StackTrace)); + FormatStackTrace(StackTrace.get()); + return StackTrace.release(); +} //--------------------------------------------------------------------------- -UnicodeString __fastcall GetExceptionDebugInfo() +UnicodeString GetExceptionDebugInfo(Exception * E) { UnicodeString Result; - TGuard Guard(StackTraceCriticalSection.get()); - TStackTraceMap::iterator Iterator = StackTraceMap.find(GetCurrentThreadId()); - if (Iterator != StackTraceMap.end()) + if (DebugAlwaysTrue(IsInternalException(E)) && DebugAlwaysTrue(E->StackInfo != NULL)) { - TStrings * StackTrace = Iterator->second; + std::unique_ptr StackTrace(GetExceptionStackTraceStrings(E)); for (int Index = 0; Index < StackTrace->Count; Index++) { UnicodeString Frame = StackTrace->Strings[Index]; @@ -569,20 +576,19 @@ UnicodeString __fastcall GetExceptionDebugInfo() } } } - } + } } return Result; } //--------------------------------------------------------------------------- -bool __fastcall AppendExceptionStackTraceAndForget(TStrings *& MoreMessages) +bool AppendExceptionStackTrace(Exception * E, TStrings *& MoreMessages) { bool Result = false; - TGuard Guard(StackTraceCriticalSection.get()); - - TStackTraceMap::iterator Iterator = StackTraceMap.find(GetCurrentThreadId()); - if (Iterator != StackTraceMap.end()) + if (IsInternalException(E) && DebugAlwaysTrue(E->StackInfo != NULL)) { + std::unique_ptr StackTrace(GetExceptionStackTraceStrings(E)); + std::unique_ptr OwnedMoreMessages; if (MoreMessages == NULL) { @@ -595,10 +601,7 @@ bool __fastcall AppendExceptionStackTraceAndForget(TStrings *& MoreMessages) MoreMessages->Text = MoreMessages->Text + "\n"; } MoreMessages->Text = MoreMessages->Text + LoadStr(STACK_TRACE) + "\n"; - MoreMessages->AddStrings(Iterator->second); - - delete Iterator->second; - StackTraceMap.erase(Iterator); + MoreMessages->AddStrings(StackTrace.get()); OwnedMoreMessages.release(); } @@ -628,7 +631,7 @@ unsigned int __fastcall ExceptionMessageDialog(Exception * E, TQueryType Type, HelpKeyword = MergeHelpKeyword(HelpKeyword, GetExceptionHelpKeyword(E)); std::unique_ptr OwnedMoreMessages; - if (AppendExceptionStackTraceAndForget(MoreMessages)) + if (AppendExceptionStackTrace(E, MoreMessages)) { OwnedMoreMessages.reset(MoreMessages); } @@ -661,45 +664,6 @@ unsigned int __fastcall FatalExceptionMessageDialog( } //--------------------------------------------------------------------------- //--------------------------------------------------------------------------- -static void __fastcall DoExceptNotify(TObject * ExceptObj, void * ExceptAddr, - bool OSException, void * BaseOfStack) -{ - if (ExceptObj != NULL) - { - Exception * E = dynamic_cast(ExceptObj); - if ((E != NULL) && IsInternalException(E)) - { - DoExceptionStackTrace(ExceptObj, ExceptAddr, OSException, BaseOfStack); - - TJclStackInfoList * StackInfoList = JclLastExceptStackList(); - - if (DebugAlwaysTrue(StackInfoList != NULL)) - { - std::unique_ptr StackTrace(StackInfoListToStrings(StackInfoList)); - - DWORD ThreadID = GetCurrentThreadId(); - - TGuard Guard(StackTraceCriticalSection.get()); - - TStackTraceMap::iterator Iterator = StackTraceMap.find(ThreadID); - if (Iterator != StackTraceMap.end()) - { - Iterator->second->Add(L""); - Iterator->second->AddStrings(StackTrace.get()); - } - else - { - StackTraceMap.insert(std::make_pair(ThreadID, StackTrace.release())); - } - - // this chains so that JclLastExceptStackList() returns NULL the next time - // for the current thread - delete StackInfoList; - } - } - } -} -//--------------------------------------------------------------------------- void * __fastcall BusyStart() { void * Token = reinterpret_cast(Screen->Cursor); @@ -1500,7 +1464,8 @@ void __fastcall WinInitialize() if (JclHookExceptions()) { JclStackTrackingOptions << stAllModules; - JclAddExceptNotifier(DoExceptNotify, npFirstChain); + // See also StackInfoListToStrings + JclExceptionStacktraceOptions >> estoIncludeAdressOffset; CallstackThread.reset(new TCallstackThread()); CallstackThread->Start(); } @@ -1514,7 +1479,6 @@ void __fastcall WinInitialize() void __fastcall WinFinalize() { CallstackThread.reset(NULL); - JclRemoveExceptNotifier(DoExceptNotify); } //--------------------------------------------------------------------------- __fastcall ::TTrayIcon::TTrayIcon(unsigned int Id)