Feedback-ID: ie3de48e3:Fastmail
Date: Mon, 9 Feb 2026 18:17:25 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Bryan Green <dbryan.green@gmail.com>
Cc: Euler Taveira <euler@eulerto.com>,
	Jakub Wartak <jakub.wartak@enterprisedb.com>,
 Michael Paquier <michael@paquier.xyz>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
Message-ID: <202602091656.sxosgjxnc42l@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="66lzhokyymna6c2w"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <f6b7eaf0-0bda-414d-962f-84c30713e390@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202602091656.sxosgjxnc42l%40alvherre.pgsql>
Precedence: bulk


--66lzhokyymna6c2w
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2025-Nov-01, Bryan Green wrote:

> Done. Now the code prints symbol+offset+address first, then conditionally
> appends line info if both SymGetLineFromAddrW64 succeeds and the filename
> conversion succeeds. This eliminates the duplication.

Thanks!

I was a bit surprised that you were doing elog(WARNING) for some
problematic conditions when trying to write the backtrace.  I think that
would work fine, because our elog.c stuff is all supposed to be
reentrant ... yet I think it's going to be odd (assuming it ever
happens).  I think we should instead just print the diagnostics message
to the backtrace string, where it will be displayed together with the
main error being processed, in the place where the backtrace would be.

This applies particularly when SymFromAddrW() fails: instead of printing
the elog(WARNING) in a separate error entry, we would still print the
function address in the correct spot of the backtrace with a small
diagnostics about the symbol not being found.  ... I think.

What do *you* think?

I also made the backtrace_cleanup() function exist always, but on
non-win32 builds it's unused, so I tagged it as such.

Github is down at the moment, so I don't know if this actually compiles.

0001 is your patch (I may have pgindented it, not sure), 0002 are my
changes.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

--66lzhokyymna6c2w
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="v7-0001-Add-backtrace-support-for-Windows-using-DbgHelp-A.patch"

From f4266b40d9e8609c2c28594650c324fc6f63219a Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Sat, 1 Nov 2025 23:04:24 -0600
Subject: [PATCH v7 1/2] Add backtrace support for Windows using DbgHelp API

Previously, backtrace generation on Windows would return an "unsupported"
message. This patch implements Windows backtrace support using
CaptureStackBackTrace() for capturing the call stack and the DbgHelp API
(SymFromAddrW, SymGetLineFromAddrW64) for symbol resolution.

The implementation provides symbol names, offsets, and addresses. When PDB
files are available, it also includes source file names and line numbers.
Symbol names and file paths are converted from UTF-16 to the database
encoding using wchar2char(), which properly handles both UTF-8 and non-UTF-8
databases on Windows. When symbol information is unavailable or encoding
conversion fails, it falls back to displaying raw addresses.

The implementation uses the explicit Unicode versions of the DbgHelp
functions (SYMBOL_INFOW, SymFromAddrW, IMAGEHLP_LINEW64, SymGetLineFromAddrW64)
rather than the generic versions. This is necessary because the generic
SYMBOL_INFO becomes SYMBOL_INFOA on PostgreSQL's Windows builds (which don't
define UNICODE), providing strings in the Windows ANSI codepage rather than
a predictable encoding that can be reliably converted to the database
encoding.

Symbol handler initialization (SymInitialize) is performed once per process
and cached. If initialization fails, a warning is logged and no backtrace is
generated. The symbol handler is cleaned up via on_proc_exit() to release
DbgHelp resources.

Author: Bryan Green
Reviewed-by: Euler Taveira, Jakub Wartak
---
 src/backend/meson.build        |   5 ++
 src/backend/utils/error/elog.c | 143 ++++++++++++++++++++++++++++++++-
 2 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/src/backend/meson.build b/src/backend/meson.build
index 712a857cdb4..2e7f2be2c78 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -1,6 +1,11 @@
 # Copyright (c) 2022-2026, PostgreSQL Global Development Group
 
 backend_build_deps = [backend_code]
+
+if host_system == 'windows' and cc.get_id() == 'msvc'
+  backend_build_deps += cc.find_library('dbghelp')
+endif
+
 backend_sources = []
 backend_link_with = [pgport_srv, common_srv]
 
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 129906e2daa..60f95a58f7a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -66,11 +66,14 @@
 #include <execinfo.h>
 #endif
 
+#ifdef _MSC_VER
+#include <dbghelp.h>
+#endif
+
 #include "access/xact.h"
 #include "common/ip.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
-#include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/miscnodes.h"
 #include "pgstat.h"
@@ -140,6 +143,11 @@ static void write_syslog(int level, const char *line);
 static void write_eventlog(int level, const char *line, int len);
 #endif
 
+#ifdef _MSC_VER
+static bool backtrace_symbols_initialized = false;
+static HANDLE backtrace_process = NULL;
+#endif
+
 /* We provide a small stack of ErrorData records for re-entrant cases */
 #define ERRORDATA_STACK_SIZE  5
 
@@ -1117,11 +1125,30 @@ errbacktrace(void)
 	return 0;
 }
 
+#ifdef _MSC_VER
+/*
+ * Cleanup function for DbgHelp resources.
+ * Called via on_proc_exit() to release resources allocated by SymInitialize().
+ */
+static void
+backtrace_cleanup(int code, Datum arg)
+{
+	SymCleanup(backtrace_process);
+}
+#endif
+
 /*
  * Compute backtrace data and add it to the supplied ErrorData.  num_skip
  * specifies how many inner frames to skip.  Use this to avoid showing the
  * internal backtrace support functions in the backtrace.  This requires that
  * this and related functions are not inlined.
+ *
+ * Platform-specific implementations:
+ * - Unix/Linux: Uses backtrace() and backtrace_symbols()
+ * - Windows: Uses CaptureStackBackTrace() with DbgHelp for symbol resolution
+ * 	 (requires PDB files; falls back to exported functions/raw addresses if
+ * 	 unavailable)
+ * - Other: Returns unsupported message
  */
 static void
 set_backtrace(ErrorData *edata, int num_skip)
@@ -1148,6 +1175,120 @@ set_backtrace(ErrorData *edata, int num_skip)
 			appendStringInfoString(&errtrace,
 								   "insufficient memory for backtrace generation");
 	}
+#elif defined(_MSC_VER)
+	{
+		void	   *buf[100];
+		int			nframes;
+		char		buffer[sizeof(SYMBOL_INFOW) + MAX_SYM_NAME * sizeof(wchar_t)];
+		PSYMBOL_INFOW psymbol;
+
+		if (!backtrace_symbols_initialized)
+		{
+			backtrace_process = GetCurrentProcess();
+
+			SymSetOptions(SYMOPT_UNDNAME |
+						  SYMOPT_DEFERRED_LOADS |
+						  SYMOPT_LOAD_LINES |
+						  SYMOPT_FAIL_CRITICAL_ERRORS);
+
+			if (SymInitialize(backtrace_process, NULL, TRUE))
+			{
+				backtrace_symbols_initialized = true;
+				on_proc_exit(backtrace_cleanup, 0);
+			}
+			else
+			{
+				elog(WARNING, "could not initialize the symbol handler: error code %lu",
+					 GetLastError());
+				edata->backtrace = errtrace.data;
+				return;
+			}
+		}
+
+		nframes = CaptureStackBackTrace(num_skip, lengthof(buf), buf, NULL);
+
+		if (nframes == 0)
+		{
+			appendStringInfoString(&errtrace, "\nNo stack frames captured");
+			edata->backtrace = errtrace.data;
+			return;
+		}
+
+		psymbol = (PSYMBOL_INFOW) buffer;
+		psymbol->MaxNameLen = MAX_SYM_NAME;
+		psymbol->SizeOfStruct = sizeof(SYMBOL_INFOW);
+
+		for (int i = 0; i < nframes; i++)
+		{
+			DWORD64		address = (DWORD64) buf[i];
+			DWORD64		displacement = 0;
+			BOOL		sym_result;
+
+			sym_result = SymFromAddrW(backtrace_process,
+									  address,
+									  &displacement,
+									  psymbol);
+			if (sym_result)
+			{
+				char		symbol_name[MAX_SYM_NAME];
+				size_t		result;
+
+				/*
+				 * Convert symbol name from UTF-16 to database encoding using
+				 * wchar2char(), which handles both UTF-8 and non-UTF-8
+				 * databases correctly on Windows.
+				 */
+				result = wchar2char(symbol_name, (const wchar_t *) psymbol->Name,
+									sizeof(symbol_name), NULL);
+
+				if (result == (size_t) -1)
+				{
+					/* Conversion failed, use address only */
+					appendStringInfo(&errtrace,
+									 "\n[0x%llx]",
+									 (unsigned long long) address);
+				}
+				else
+				{
+					IMAGEHLP_LINEW64 line;
+					DWORD		line_displacement = 0;
+					char		filename[MAX_PATH];
+
+					line.SizeOfStruct = sizeof(IMAGEHLP_LINEW64);
+
+					/* Start with the common part: symbol+offset [address] */
+					appendStringInfo(&errtrace,
+									 "\n%s+0x%llx [0x%llx]",
+									 symbol_name,
+									 (unsigned long long) displacement,
+									 (unsigned long long) address);
+
+					/* Try to append line info if available */
+					if (SymGetLineFromAddrW64(backtrace_process,
+											  address,
+											  &line_displacement,
+											  &line))
+					{
+						result = wchar2char(filename, (const wchar_t *) line.FileName,
+											sizeof(filename), NULL);
+
+						if (result != (size_t) -1)
+						{
+							appendStringInfo(&errtrace,
+											 " [%s:%lu]",
+											 filename,
+											 (unsigned long) line.LineNumber);
+						}
+					}
+				}
+			}
+			else
+			{
+				elog(WARNING, "symbol lookup failed: error code %lu",
+					 GetLastError());
+			}
+		}
+	}
 #else
 	appendStringInfoString(&errtrace,
 						   "backtrace generation is not supported by this installation");
-- 
2.47.3


--66lzhokyymna6c2w
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment; filename="v7-0002-fixups.patch"

From d1f588ab0b670f677eaa844e68b7e790d6f03c51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Mon, 9 Feb 2026 17:51:51 +0100
Subject: [PATCH v7 2/2] fixups

---
 src/backend/meson.build        | 10 +++---
 src/backend/utils/error/elog.c | 57 ++++++++++++++++++----------------
 2 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/src/backend/meson.build b/src/backend/meson.build
index 2e7f2be2c78..1ee2c079390 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -2,10 +2,6 @@
 
 backend_build_deps = [backend_code]
 
-if host_system == 'windows' and cc.get_id() == 'msvc'
-  backend_build_deps += cc.find_library('dbghelp')
-endif
-
 backend_sources = []
 backend_link_with = [pgport_srv, common_srv]
 
@@ -46,6 +42,12 @@ backend_link_args = []
 backend_link_depends = []
 
 
+# On Windows also make the backend depend on dbghelp, for backtrace support
+if host_system == 'windows' and cc.get_id() == 'msvc'
+  backend_build_deps += cc.find_library('dbghelp')
+endif
+
+
 # On windows when compiling with msvc we need to make postgres export all its
 # symbols so that extension libraries can use them. For that we need to scan
 # the constituting objects and generate a file specifying all the functions as
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 60f95a58f7a..6b80e44fb5d 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -74,6 +74,7 @@
 #include "common/ip.h"
 #include "libpq/libpq.h"
 #include "libpq/pqformat.h"
+#include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/miscnodes.h"
 #include "pgstat.h"
@@ -188,6 +189,7 @@ static void set_stack_entry_location(ErrorData *edata,
 									 const char *funcname);
 static bool matches_backtrace_functions(const char *funcname);
 static pg_noinline void set_backtrace(ErrorData *edata, int num_skip);
+static void backtrace_cleanup(int code, Datum arg);
 static void set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str);
 static void FreeErrorDataContents(ErrorData *edata);
 static int	log_min_messages_cmp(const ListCell *a, const ListCell *b);
@@ -1125,30 +1127,17 @@ errbacktrace(void)
 	return 0;
 }
 
-#ifdef _MSC_VER
-/*
- * Cleanup function for DbgHelp resources.
- * Called via on_proc_exit() to release resources allocated by SymInitialize().
- */
-static void
-backtrace_cleanup(int code, Datum arg)
-{
-	SymCleanup(backtrace_process);
-}
-#endif
-
 /*
  * Compute backtrace data and add it to the supplied ErrorData.  num_skip
  * specifies how many inner frames to skip.  Use this to avoid showing the
  * internal backtrace support functions in the backtrace.  This requires that
  * this and related functions are not inlined.
  *
- * Platform-specific implementations:
- * - Unix/Linux: Uses backtrace() and backtrace_symbols()
+ * The implementation is, unsurprisingly, platform-specific:
+ * - Linux, Unix: Uses backtrace() and backtrace_symbols()
  * - Windows: Uses CaptureStackBackTrace() with DbgHelp for symbol resolution
  * 	 (requires PDB files; falls back to exported functions/raw addresses if
  * 	 unavailable)
- * - Other: Returns unsupported message
  */
 static void
 set_backtrace(ErrorData *edata, int num_skip)
@@ -1159,12 +1148,12 @@ set_backtrace(ErrorData *edata, int num_skip)
 
 #ifdef HAVE_BACKTRACE_SYMBOLS
 	{
-		void	   *buf[100];
+		void	   *frames[100];
 		int			nframes;
 		char	  **strfrms;
 
-		nframes = backtrace(buf, lengthof(buf));
-		strfrms = backtrace_symbols(buf, nframes);
+		nframes = backtrace(frames, lengthof(frames));
+		strfrms = backtrace_symbols(frames, nframes);
 		if (strfrms != NULL)
 		{
 			for (int i = num_skip; i < nframes; i++)
@@ -1177,7 +1166,7 @@ set_backtrace(ErrorData *edata, int num_skip)
 	}
 #elif defined(_MSC_VER)
 	{
-		void	   *buf[100];
+		void	   *frames[100];
 		int			nframes;
 		char		buffer[sizeof(SYMBOL_INFOW) + MAX_SYM_NAME * sizeof(wchar_t)];
 		PSYMBOL_INFOW psymbol;
@@ -1198,18 +1187,19 @@ set_backtrace(ErrorData *edata, int num_skip)
 			}
 			else
 			{
-				elog(WARNING, "could not initialize the symbol handler: error code %lu",
-					 GetLastError());
+				appendStringInfo(&errtrace,
+								 "could not initialize symbol handler: error code %lu",
+								 GetLastError());
 				edata->backtrace = errtrace.data;
 				return;
 			}
 		}
 
-		nframes = CaptureStackBackTrace(num_skip, lengthof(buf), buf, NULL);
+		nframes = CaptureStackBackTrace(num_skip, lengthof(frames), frames, NULL);
 
 		if (nframes == 0)
 		{
-			appendStringInfoString(&errtrace, "\nNo stack frames captured");
+			appendStringInfoString(&errtrace, "zero stack frames captured");
 			edata->backtrace = errtrace.data;
 			return;
 		}
@@ -1220,7 +1210,7 @@ set_backtrace(ErrorData *edata, int num_skip)
 
 		for (int i = 0; i < nframes; i++)
 		{
-			DWORD64		address = (DWORD64) buf[i];
+			DWORD64		address = (DWORD64) frames[i];
 			DWORD64		displacement = 0;
 			BOOL		sym_result;
 
@@ -1284,8 +1274,10 @@ set_backtrace(ErrorData *edata, int num_skip)
 			}
 			else
 			{
-				elog(WARNING, "symbol lookup failed: error code %lu",
-					 GetLastError());
+				appendStringInfo(&errtrace,
+								 "\n[0x%llx] (symbol lookup failed: error code %lu)",
+								 (unsigned long long) address,
+								 GetLastError());
 			}
 		}
 	}
@@ -1297,6 +1289,19 @@ set_backtrace(ErrorData *edata, int num_skip)
 	edata->backtrace = errtrace.data;
 }
 
+/*
+ * Cleanup function for DbgHelp resources.
+ * Called via on_proc_exit() to release resources allocated by SymInitialize().
+ */
+pg_attribute_unused()
+static void
+backtrace_cleanup(int code, Datum arg)
+{
+#ifdef _MSC_VER
+	SymCleanup(backtrace_process);
+#endif
+}
+
 /*
  * errmsg_internal --- add a primary error message text to the current error
  *
-- 
2.47.3


--66lzhokyymna6c2w--