Fixing stacktrace collection for abnormal program termination for 64-bit build

Source commit: d211d0525413d5b9a06ea25c788fa3d1377c68f3

source/WinSCP.cpp

@@ -29,7 +29,7 @@ void AbortSignalHandler(int Signal)
   std::unique_ptr<TStrings> MoreMessages(new TStringList());
   UnicodeString ReportErrorText = Trim(FMTLOAD(REPORT_ERROR, (EmptyStr)));
   MoreMessages->Add(ReportErrorText);
-  std::unique_ptr<TStrings> StackTrace(GetCurrentStackTrace());
+  std::unique_ptr<TStrings> StackTrace(GetAbortStackTrace());
   AppendStackTrace(MoreMessages.get(), StackTrace.get());
   MoreMessageDialog(Message, MoreMessages.get(), qtError, qaOK | qaReport, EmptyStr);
 }

source/core/Interface.h

@@ -42,6 +42,7 @@ void __fastcall ShowExtendedException(Exception * E);
 void AppendStackTrace(TStrings * MoreMessages, TStrings * StackTrace);
 bool AppendExceptionStackTrace(Exception * E, TStrings *& MoreMessages);
 TStrings * GetCurrentStackTrace();
+TStrings * GetAbortStackTrace();
 void __fastcall IgnoreException(const std::type_info & ExceptionType);
 UnicodeString GetExceptionDebugInfo(Exception * E);
 

source/packages/jcl/JclDebug.pas

@@ -1456,6 +1456,7 @@ class function TJclAbstractMapParser.MapStringToStr(MapString: PJclMapString;
   IgnoreSpaces: Boolean): string;
 var
   P: PJclMapString;
+  TemplateDepth: Integer; // WINSCP
 begin
   if MapString = nil then
   begin
@@ -1472,12 +1473,23 @@ begin
   else
   begin
     P := MapString;
-    if IgnoreSpaces then
-      while (P^ <> #0) and not (P^ in ['(', #10, #13]) do
-        Inc(P)
-    else
-      while (P^ <> #0) and (P^ <> '(') and (P^ > ' ') do
-        Inc(P);
+    // WINSCP
+    TemplateDepth := 0;
+    while P^ <> #0 do
+    begin
+      if (P^ in [#10, #13]) or ((not IgnoreSpaces) and (P^ <= ' ')) then
+        Break;
+
+      if P^ = '<' then
+        Inc(TemplateDepth)
+      else if P^ = '>' then
+        Dec(TemplateDepth);
+
+      if (P^ = '(') and (TemplateDepth <= 0) then
+        Break;
+
+      Inc(P);
+    end;
   end;
   SetString(Result, MapString, P - MapString);
 end;

source/windows/WinInterface.cpp

@@ -617,11 +617,11 @@ static void FormatStackTrace(TStrings * StackTrace)
   }
 }
 //---------------------------------------------------------------------------
-static TStrings * __fastcall StackInfoListToStrings(
-  TJclStackInfoList * StackInfoList)
+static TStrings * __fastcall StackInfoListToStrings(TJclStackInfoList * StackInfoList)
 {
   std::unique_ptr<TStrings> StackTrace(new TStringList());
-  // Corresponds to our JclExceptionStacktraceOptions
+  // Corresponds to our JclExceptionStacktraceOptions.
+  // Also match GetLocationInfoStr call in GetAbortStackTrace.
   StackInfoList->AddToStrings(StackTrace.get(), true, false, true, true);
   FormatStackTrace(StackTrace.get());
   return StackTrace.release();
@@ -762,28 +762,124 @@ unsigned int __fastcall FatalExceptionMessageDialog(
   return ExceptionMessageDialog(E, Type, MessageFormat, Answers, HelpKeyword, &AParams);
 }
 //---------------------------------------------------------------------------
+static void FilterStackTrace(TStrings * StackTrace)
+{
+  for (int Index = 0; Index < StackTrace->Count; Index++)
+  {
+    UnicodeString Frame = StackTrace->Strings[Index];
+    UnicodeString Symbol = GetFrameSymbol(Frame);
+    if ((Symbol == L"GetCurrentStackTrace") ||
+        (Symbol == L"GetAbortStackTrace") ||
+        (Symbol == L"Jcldebug::JclCreateStackList") ||
+        (Symbol == L"AbortSignalHandler") ||
+        (Symbol == L"TerminateHandler") ||
+        (Symbol == L"_raise") ||
+        (Symbol == L"raise") ||
+        (Symbol == L"abort") ||
+        (Symbol == L"std::terminate") ||
+        (Symbol == L"___cpp_terminate"))
+    {
+      StackTrace->Delete(Index);
+      Index--;
+    }
+  }
+}
+//---------------------------------------------------------------------------
 TStrings * GetCurrentStackTrace()
 {
   std::unique_ptr<TJclStackInfoList> StackInfoList(JclCreateStackList(false, 0, Caller(0, false)));
   std::unique_ptr<TStrings> Result(StackInfoListToStrings(StackInfoList.get()));
-  for (int Index = 0; Index < Result->Count; Index++)
-  {
-    UnicodeString Frame = Result->Strings[Index];
-    UnicodeString Symbol = GetFrameSymbol(Frame);
-    if ((Symbol == L"GetCurrentStackTrace") ||
-        (Symbol == L"Jcldebug::JclCreateStackList") ||
-        (Symbol == L"AbortSignalHandler") ||
-        (Symbol == L"_raise") ||
-        (Symbol == L"std::terminate") ||
-        (Symbol == L"___cpp_terminate"))
-    {
-      Result->Delete(Index);
-      Index--;
-    }
-  }
+  FilterStackTrace(Result.get());
   return Result.release();
 }
 //---------------------------------------------------------------------------
+#ifdef _WIN64
+// This is what TJclStackInfoList.ValidCallSite should do, had it been implemented for 64-bit. Maybe one day it will.
+bool IsValidCallSite64(ULONG_PTR CodeAddr)
+{
+  if (CodeAddr <= 8) return false;
+
+  uint8_t* P = reinterpret_cast<uint8_t*>(CodeAddr);
+
+  if (IsBadReadPtr(P - 8, 8)) return false;
+
+  // 1. CALL rel32 (E8 xx xx xx xx) -> 5 bytes
+  if (P[-5] == 0xE8) return true;
+
+  // 2. Indirect CALLs (FF /2) with or without REX prefixes (40-4F)
+
+  // CALL reg (FF D0-D7) -> 2 bytes
+  if (P[-2] == 0xFF && (P[-1] >= 0xD0 && P[-1] <= 0xD7)) return true;
+  // CALL reg with REX prefix (e.g. CALL R11) -> 3 bytes
+  if (P[-3] >= 0x40 && P[-3] <= 0x4F && P[-2] == 0xFF && (P[-1] >= 0xD0 && P[-1] <= 0xD7)) return true;
+
+  // CALL [RIP+disp32] (FF 15 xx xx xx xx) -> 6 bytes
+  if (P[-6] == 0xFF && P[-5] == 0x15) return true;
+
+  // CALL [reg+disp8] (FF 50-57) -> 3 bytes
+  if (P[-3] == 0xFF && (P[-2] >= 0x50 && P[-2] <= 0x57)) return true;
+  // CALL [reg+disp8] with REX -> 4 bytes
+  if (P[-4] >= 0x40 && P[-4] <= 0x4F && P[-3] == 0xFF && (P[-2] >= 0x50 && P[-2] <= 0x57)) return true;
+
+  // CALL [reg] (FF 10-17) -> 2 bytes
+  if (P[-2] == 0xFF && (P[-1] >= 0x10 && P[-1] <= 0x17)) return true;
+  // CALL [reg] with REX -> 3 bytes
+  if (P[-3] >= 0x40 && P[-3] <= 0x4F && P[-2] == 0xFF && (P[-1] >= 0x10 && P[-1] <= 0x17)) return true;
+
+  return false;
+}
+#endif
+//---------------------------------------------------------------------------
+TStrings * GetAbortStackTrace()
+{
+  // In 64-bit when abort handlers is called, the stack is already unwind, so we canot use JclCreateStackList,
+  // instread we walk stale raw stack data.
+  #ifdef _WIN64
+  std::unique_ptr<TStrings> Result(new TStringList());
+  CONTEXT Context;
+  ZeroMemory(&Context, sizeof(Context));
+  Context.ContextFlags = CONTEXT_FULL;
+  RtlCaptureContext(&Context);
+
+  // Define the stack boundaries
+  // RSP is the current pointer; the Thread Environment Block (TEB) holds the absolute base.
+  ULONG_PTR * CurrentStack = reinterpret_cast<ULONG_PTR*>(Context.Rsp);
+  PNT_TIB Tib = reinterpret_cast<PNT_TIB>(NtCurrentTeb());
+  ULONG_PTR * StackBase = reinterpret_cast<ULONG_PTR*>(Tib->StackBase);
+
+  // Scan the raw stack memory upward
+  while (CurrentStack < StackBase)
+  {
+    ULONG_PTR PotentialAddr = *CurrentStack;
+    void * Addr = reinterpret_cast<void*>(PotentialAddr);
+
+    // Use JCL to verify if this pointer belongs to a loaded executable module
+    if (JclValidateModuleAddress(Addr) &&
+        IsValidCallSite64(PotentialAddr))
+    {
+      // If JCL can map this address to a function, log it!
+      TJclLocationInfo Info;
+      if (GetLocationInfo(Addr, Info))
+      {
+        // Parameters match AddToStrings call in StackInfoListToStrings
+        String TraceLine = GetLocationInfoStr(Addr, true, false, true, true);
+        // Deduplicate identical sequential lines
+        if ((Result->Count == 0) || (Result->Strings[Result->Count - 1] != TraceLine))
+        {
+          Result->Add(TraceLine);
+        }
+      }
+    }
+    CurrentStack++;
+  }
+  FormatStackTrace(Result.get());
+  FilterStackTrace(Result.get());
+  return Result.release();
+  #else
+  return GetCurrentStackTrace();
+  #endif
+}
+//---------------------------------------------------------------------------
 //---------------------------------------------------------------------------
 void * __fastcall BusyStart()
 {