agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Bryan Green <dbryan.green@gmail.com>
To: Jakub Wartak <jakub.wartak@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
Date: Tue, 21 Oct 2025 20:09:56 -0500
Message-ID: <6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com> (raw)
In-Reply-To: <CAKZiRmzxyNFGQEM=v1KaN3F3xgn1XYN2Udw+ghyT+sVA_impsg@mail.gmail.com>
References: <a692c0fe-caca-4c08-9c5d-debfd0ef2504@gmail.com>
<CAKZiRmzxyNFGQEM=v1KaN3F3xgn1XYN2Udw+ghyT+sVA_impsg@mail.gmail.com>
On 10/20/2025 3:10 AM, Jakub Wartak wrote:
> On Thu, Oct 9, 2025 at 5:50 PM Bryan Green <dbryan.green@gmail.com> wrote:
>
> Hi Bryan!
>
>> I've implemented Windows support for the backtrace_functions configuration parameter using the DbgHelp API. This brings Windows debugging capabilities closer to parity with Unix/Linux platforms.
>
> Cool, thanks for working on this. Win32 is a bit alien to me, but I've
> got access to win32 so I've played with the patch a little bit. With
> simple: SET backtrace_functions = 'typenameType'; CREATE TABLE tab (id
> invalidtype);
>
> I've got
> 2025-10-20 08:47:25.440 CEST [25700] ERROR: type "invalidtype"
> does not exist at character 22
> 2025-10-20 08:47:25.440 CEST [25700] BACKTRACE:
> typenameType+0x86
> [C:\git\postgres-master\src\backend\parser\parse_type.c:270]
> transformColumnDefinition+0x1df
> [C:\git\postgres-master\src\backend\parser\parse_utilcmd.c:649]
> transformCreateStmt+0x306
> [C:\git\postgres-master\src\backend\parser\parse_utilcmd.c:279]
> [..]
> main+0x4f8 [C:\git\postgres-master\src\backend\main\main.c:218]
> __scrt_common_main_seh+0x10c
> [D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288]
> BaseThreadInitThunk+0x17 [0x7ffc5f06e8d7]
> RtlUserThreadStart+0x2c [0x7ffc5ff08d9c]
> 2025-10-20 08:47:25.440 CEST [25700] STATEMENT: CREATE TABLE tab
> (id invalidtype);
>
>> Without PDB files, raw memory addresses are shown.
>
> and after removing PDB files, so it's how the real distribution of
> Win32 are shipped, it's still showing function names (but MSVC
> shouldn't be putting symbols into binaries, right? I mean it's better
> than I expected):
MSVC does put function names into release builds. You should see
function names, raw addresses, NO file paths or line numbers in a
release build. PDB files would give you the file paths and line
numbers.> 2025-10-20 09:49:03.061 CEST [22832] ERROR: type
"invalidtype"
> does not exist at character 22
> 2025-10-20 09:49:03.061 CEST [22832] BACKTRACE:
> typenameType+0x86 [0x7ff71e6e3426]
> transformAlterTableStmt+0xb7f [0x7ff71e6e5eef]
> transformCreateStmt+0x306 [0x7ff71e6e78a6]
> [..]
>
> meanwhile on Linux:
> 2025-10-20 08:49:11.758 CEST [4310] ERROR: type "invalidtype"
> does not exist at character 22
> 2025-10-20 08:49:11.758 CEST [4310] BACKTRACE:
> postgres: postgres postgres [local] CREATE
> TABLE(typenameType+0x86) [0x560f082eeb06]
> postgres: postgres postgres [local] CREATE
> TABLE(+0x36ae37) [0x560f082efe37]
> postgres: postgres postgres [local] CREATE
> TABLE(transformCreateStmt+0x386) [0x560f082ef676]
> [..]
>
> Clearly the MSVC version by default seems to be more reliable in terms
> of symbols resolution. Anyway:
>
> 1. Should outputs be aligned in terms of process title, so should we
> aim with this $patch to also include the full process name (like
> `postgres: postgres postgres [local]` above and often the operation
> `CREATE TABLE` too) which seems to be coming from setproctitle(3bsd)
> and does depend on update_process_title GUC. However on win32 this is
> off (so nobody will touch it), because docs say 'This setting defaults
> to on on most platforms, but it defaults to off on Windows due to that
> platform's larger overhead for updating the process title'. IMHO it is
> good as it is, which is to have something rather than nothing.
> Personally for me it is pretty strange that original
> backtrace_symbols(3) includes './progname' in the output on Linux, but
> it is what we have today.
>
I think it is good as is.> 2. Code review itself:
>
> a. nitpicking nearby:
> + * - Unix/Linux/: Uses backtrace() and backtrace_symbols() <--
> git diff shows there's unnecessary empty space at the end
>
Thanks for catching this.>> Confirmed no crashes or memory leaks
>
> b. Shouldn't we call SymCleanup(process) at some point to free
> resources anyway? (I'm wondering about the scenario in case a very
> long-lived process hits $backtrace_functions every couple of seconds -
> wouldn't that cause a leak over a very long term without SymCleanup()
> ?)
>
> -J.
Yes. We should call cleanup at the backend shutdown, as initialize is
called once. I have put together a new patch (for better patch naming)
and added the cleanup code.
BG
From 81ee688f490fa37b5eb30d1d88123e3d0a8423f2 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Tue, 21 Oct 2025 19:31:29 -0500
Subject: [PATCH] Add Windows support for backtrace_functions
Implement backtrace generation on Windows using the DbgHelp API, providing
similar functionality to the existing Unix/Linux support. When PDB files
are available, backtraces include function names and source locations.
Without PDB files, raw addresses are shown.
DbgHelp is initialized once per backend and cleaned up via on_proc_exit().
This adds a dependency on dbghelp.lib for MSVC builds.
---
src/backend/meson.build | 5 +
src/backend/utils/error/elog.c | 174 +++++++++++++++++++++++++++++++++
2 files changed, 179 insertions(+)
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b831a54165..eeb69c4079 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -1,6 +1,11 @@
# Copyright (c) 2022-2025, 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 29643c5143..fc421ce444 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -140,6 +140,13 @@ static void write_syslog(int level, const char *line);
static void write_eventlog(int level, const char *line, int len);
#endif
+#ifdef _MSC_VER
+#include <windows.h>
+#include <dbghelp.h>
+static bool win32_backtrace_symbols_initialized = false;
+static HANDLE win32_backtrace_process = NULL;
+#endif
+
/* We provide a small stack of ErrorData records for re-entrant cases */
#define ERRORDATA_STACK_SIZE 5
@@ -1116,6 +1123,18 @@ 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
+win32_backtrace_cleanup(int code, Datum arg)
+{
+ SymCleanup(win32_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
@@ -1147,6 +1166,161 @@ set_backtrace(ErrorData *edata, int num_skip)
appendStringInfoString(&errtrace,
"insufficient memory for backtrace generation");
}
+#elif defined(_MSC_VER)
+ {
+ void *stack[100];
+ DWORD frames;
+ DWORD i;
+ wchar_t buffer[sizeof(SYMBOL_INFOW) + MAX_SYM_NAME * sizeof(wchar_t)];
+ PSYMBOL_INFOW symbol;
+ char *utf8_buffer;
+ int utf8_len;
+
+ if (!win32_backtrace_symbols_initialized)
+ {
+ win32_backtrace_process = GetCurrentProcess();
+
+ SymSetOptions(SYMOPT_UNDNAME |
+ SYMOPT_DEFERRED_LOADS |
+ SYMOPT_LOAD_LINES |
+ SYMOPT_FAIL_CRITICAL_ERRORS);
+
+ if (SymInitialize(win32_backtrace_process, NULL, TRUE))
+ {
+ win32_backtrace_symbols_initialized = true;
+ on_proc_exit(win32_backtrace_cleanup, 0);
+ }
+ else
+ {
+ DWORD error = GetLastError();
+
+ elog(WARNING, "SymInitialize failed with error %lu", error);
+ }
+ }
+
+ frames = CaptureStackBackTrace(num_skip, lengthof(stack), stack, NULL);
+
+ if (frames == 0)
+ {
+ appendStringInfoString(&errtrace, "\nNo stack frames captured");
+ edata->backtrace = errtrace.data;
+ return;
+ }
+
+ symbol = (PSYMBOL_INFOW) buffer;
+ symbol ->MaxNameLen = MAX_SYM_NAME;
+ symbol ->SizeOfStruct = sizeof(SYMBOL_INFOW);
+
+ for (i = 0; i < frames; i++)
+ {
+ DWORD64 address = (DWORD64) (stack[i]);
+ DWORD64 displacement = 0;
+ BOOL sym_result;
+
+ sym_result = SymFromAddrW(win32_backtrace_process,
+ address,
+ &displacement,
+ symbol);
+
+ if (sym_result)
+ {
+ IMAGEHLP_LINEW64 line;
+ DWORD line_displacement = 0;
+
+ line.SizeOfStruct = sizeof(IMAGEHLP_LINEW64);
+
+ if (SymGetLineFromAddrW64(win32_backtrace_process,
+ address,
+ &line_displacement,
+ &line))
+ {
+ /* Convert symbol name to UTF-8 */
+ utf8_len = WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ NULL, 0, NULL, NULL);
+ if (utf8_len > 0)
+ {
+ char *filename_utf8;
+ int filename_len;
+
+ utf8_buffer = palloc(utf8_len);
+ WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ utf8_buffer, utf8_len, NULL, NULL);
+
+ /* Convert file name to UTF-8 */
+ filename_len = WideCharToMultiByte(CP_UTF8, 0,
+ line.FileName, -1,
+ NULL, 0, NULL, NULL);
+ if (filename_len > 0)
+ {
+ filename_utf8 = palloc(filename_len);
+ WideCharToMultiByte(CP_UTF8, 0, line.FileName, -1,
+ filename_utf8, filename_len,
+ NULL, NULL);
+
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [%s:%lu]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ filename_utf8,
+ (unsigned long) line.LineNumber);
+
+ pfree(filename_utf8);
+ }
+ else
+ {
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [0x%llx]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ (unsigned long long) address);
+ }
+
+ pfree(utf8_buffer);
+ }
+ else
+ {
+ /* Conversion failed, use address only */
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ else
+ {
+ /* No line info, convert symbol name only */
+ utf8_len = WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ NULL, 0, NULL, NULL);
+ if (utf8_len > 0)
+ {
+ utf8_buffer = palloc(utf8_len);
+ WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ utf8_buffer, utf8_len, NULL, NULL);
+
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [0x%llx]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ (unsigned long long) address);
+
+ pfree(utf8_buffer);
+ }
+ else
+ {
+ /* Conversion failed, use address only */
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ }
+ else
+ {
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ }
#else
appendStringInfoString(&errtrace,
"backtrace generation is not supported by this installation");
--
2.46.0.windows.1
Attachments:
[text/plain] 0001-Add-Windows-support-for-backtrace_functions.patch (6.5K, ../6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com/2-0001-Add-Windows-support-for-backtrace_functions.patch)
download | inline diff:
From 81ee688f490fa37b5eb30d1d88123e3d0a8423f2 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Tue, 21 Oct 2025 19:31:29 -0500
Subject: [PATCH] Add Windows support for backtrace_functions
Implement backtrace generation on Windows using the DbgHelp API, providing
similar functionality to the existing Unix/Linux support. When PDB files
are available, backtraces include function names and source locations.
Without PDB files, raw addresses are shown.
DbgHelp is initialized once per backend and cleaned up via on_proc_exit().
This adds a dependency on dbghelp.lib for MSVC builds.
---
src/backend/meson.build | 5 +
src/backend/utils/error/elog.c | 174 +++++++++++++++++++++++++++++++++
2 files changed, 179 insertions(+)
diff --git a/src/backend/meson.build b/src/backend/meson.build
index b831a54165..eeb69c4079 100644
--- a/src/backend/meson.build
+++ b/src/backend/meson.build
@@ -1,6 +1,11 @@
# Copyright (c) 2022-2025, 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 29643c5143..fc421ce444 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -140,6 +140,13 @@ static void write_syslog(int level, const char *line);
static void write_eventlog(int level, const char *line, int len);
#endif
+#ifdef _MSC_VER
+#include <windows.h>
+#include <dbghelp.h>
+static bool win32_backtrace_symbols_initialized = false;
+static HANDLE win32_backtrace_process = NULL;
+#endif
+
/* We provide a small stack of ErrorData records for re-entrant cases */
#define ERRORDATA_STACK_SIZE 5
@@ -1116,6 +1123,18 @@ 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
+win32_backtrace_cleanup(int code, Datum arg)
+{
+ SymCleanup(win32_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
@@ -1147,6 +1166,161 @@ set_backtrace(ErrorData *edata, int num_skip)
appendStringInfoString(&errtrace,
"insufficient memory for backtrace generation");
}
+#elif defined(_MSC_VER)
+ {
+ void *stack[100];
+ DWORD frames;
+ DWORD i;
+ wchar_t buffer[sizeof(SYMBOL_INFOW) + MAX_SYM_NAME * sizeof(wchar_t)];
+ PSYMBOL_INFOW symbol;
+ char *utf8_buffer;
+ int utf8_len;
+
+ if (!win32_backtrace_symbols_initialized)
+ {
+ win32_backtrace_process = GetCurrentProcess();
+
+ SymSetOptions(SYMOPT_UNDNAME |
+ SYMOPT_DEFERRED_LOADS |
+ SYMOPT_LOAD_LINES |
+ SYMOPT_FAIL_CRITICAL_ERRORS);
+
+ if (SymInitialize(win32_backtrace_process, NULL, TRUE))
+ {
+ win32_backtrace_symbols_initialized = true;
+ on_proc_exit(win32_backtrace_cleanup, 0);
+ }
+ else
+ {
+ DWORD error = GetLastError();
+
+ elog(WARNING, "SymInitialize failed with error %lu", error);
+ }
+ }
+
+ frames = CaptureStackBackTrace(num_skip, lengthof(stack), stack, NULL);
+
+ if (frames == 0)
+ {
+ appendStringInfoString(&errtrace, "\nNo stack frames captured");
+ edata->backtrace = errtrace.data;
+ return;
+ }
+
+ symbol = (PSYMBOL_INFOW) buffer;
+ symbol ->MaxNameLen = MAX_SYM_NAME;
+ symbol ->SizeOfStruct = sizeof(SYMBOL_INFOW);
+
+ for (i = 0; i < frames; i++)
+ {
+ DWORD64 address = (DWORD64) (stack[i]);
+ DWORD64 displacement = 0;
+ BOOL sym_result;
+
+ sym_result = SymFromAddrW(win32_backtrace_process,
+ address,
+ &displacement,
+ symbol);
+
+ if (sym_result)
+ {
+ IMAGEHLP_LINEW64 line;
+ DWORD line_displacement = 0;
+
+ line.SizeOfStruct = sizeof(IMAGEHLP_LINEW64);
+
+ if (SymGetLineFromAddrW64(win32_backtrace_process,
+ address,
+ &line_displacement,
+ &line))
+ {
+ /* Convert symbol name to UTF-8 */
+ utf8_len = WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ NULL, 0, NULL, NULL);
+ if (utf8_len > 0)
+ {
+ char *filename_utf8;
+ int filename_len;
+
+ utf8_buffer = palloc(utf8_len);
+ WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ utf8_buffer, utf8_len, NULL, NULL);
+
+ /* Convert file name to UTF-8 */
+ filename_len = WideCharToMultiByte(CP_UTF8, 0,
+ line.FileName, -1,
+ NULL, 0, NULL, NULL);
+ if (filename_len > 0)
+ {
+ filename_utf8 = palloc(filename_len);
+ WideCharToMultiByte(CP_UTF8, 0, line.FileName, -1,
+ filename_utf8, filename_len,
+ NULL, NULL);
+
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [%s:%lu]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ filename_utf8,
+ (unsigned long) line.LineNumber);
+
+ pfree(filename_utf8);
+ }
+ else
+ {
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [0x%llx]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ (unsigned long long) address);
+ }
+
+ pfree(utf8_buffer);
+ }
+ else
+ {
+ /* Conversion failed, use address only */
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ else
+ {
+ /* No line info, convert symbol name only */
+ utf8_len = WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ NULL, 0, NULL, NULL);
+ if (utf8_len > 0)
+ {
+ utf8_buffer = palloc(utf8_len);
+ WideCharToMultiByte(CP_UTF8, 0, symbol->Name, -1,
+ utf8_buffer, utf8_len, NULL, NULL);
+
+ appendStringInfo(&errtrace,
+ "\n%s+0x%llx [0x%llx]",
+ utf8_buffer,
+ (unsigned long long) displacement,
+ (unsigned long long) address);
+
+ pfree(utf8_buffer);
+ }
+ else
+ {
+ /* Conversion failed, use address only */
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ }
+ else
+ {
+ appendStringInfo(&errtrace,
+ "\n[0x%llx]",
+ (unsigned long long) address);
+ }
+ }
+ }
#else
appendStringInfoString(&errtrace,
"backtrace generation is not supported by this installation");
--
2.46.0.windows.1
view thread (36+ messages) latest in thread
Message-ID: <6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com>
Permalink: ../6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com/
Also on: postgresql.org/message-id/6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: dbryan.green@gmail.com, jakub.wartak@enterprisedb.com
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
In-Reply-To: <6c8d04fc-d87e-4391-b445-bd07d7375ca6@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox