Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)

On Thu, Oct 9, 2025 at 5:50 PM Bryan Green <dbryan(dot)green(at)gmail(dot)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):
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.

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

> 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.