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

On 11/1/2025 9:31 AM, Euler Taveira wrote:
> On Sat, Nov 1, 2025, at 1:40 AM, Bryan Green wrote:
>> v5 patch attached, incorporating all of Euler's feedback with a caveat
>> around unicode.
>>
>
> Thanks for sharing another patch.
>

Thanks for the continued review. v6 patch attached with your latest
suggestions incorporated.

>> The most interesting aspect turned out to be the encoding conversion for
>> symbol names and file paths. Initially I tried using the generic
>> SYMBOL_INFO and SymFromAddr functions as Euler suggested, but ran into a
>> subtle issue: on PostgreSQL's Windows builds, these become SYMBOL_INFOA
>> and SymFromAddrA (the ANSI versions), which return strings in whatever
>> Windows ANSI codepage happens to be active (CP1252, etc). This doesn't
>> necessarily match the database encoding.
>>
>
> Odd. Does the build define UNICODE? (Don't have a Windows machine right now to
> explore this case.)
>

PostgreSQL's Windows builds don't define UNICODE globally. I verified
this by checking that SYMBOL_INFO resolves to the ANSI version - when I
tried using it with wchar2char(), the conversion failed because the
input wasn't actually wide characters.
>> The solution was to use the explicit Unicode versions (SYMBOL_INFOW and
>> SymFromAddrW), which reliably return UTF-16 strings that wchar2char()
>> can properly convert to the database encoding. This handles both UTF-8
>> and non-UTF-8 databases correctly, and wchar2char() gracefully returns
>> -1 on conversion failure rather than throwing errors during error
>> handling. Of course this also necessitated using IMAGEHLP_LINEW64 and
>> SymGetLineFromAddrW64.
>>
>
> Works for me.
>
>> This patch also adds a check for zero frames returned by backtrace() on
>> Unix/Linux platforms, which can occur in certain circumstances such as
>> ARM builds without unwind tables.
>>
>
> Please create a separate patch.

Removed the check - I'll submit that separately.

>
> + if (result != (size_t) -1)
> + {
> + appendStringInfo(&errtrace,
> + "\n%s+0x%llx [0x%llx] [%s:%lu]",
> + symbol_name,
> + (unsigned long long) displacement,
> + (unsigned long long) address,
> + filename,
> + (unsigned long) line.LineNumber);
> + }
> + else
> + {
> + /* Filename conversion failed, omit it */
> + appendStringInfo(&errtrace,
> + "\n%s+0x%llx [0x%llx]",
> + symbol_name,
> + (unsigned long long) displacement,
> + (unsigned long long) address);
> + }
> + }
> + else
> + {
> + /* No line info available */
> + appendStringInfo(&errtrace,
> + "\n%s+0x%llx [0x%llx]",
> + symbol_name,
> + (unsigned long long) displacement,
> + (unsigned long long) address);
> + }
>
> One last suggestion is to have a single code path for these last two conditions
> to avoid duplication. Move this if to outside the "if SymGetLineFromAddrW64".
> Merge the comment to reflect both conditions (file conversion failed and no
> line info).
>
>

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.

Also added the backtrace_cleanup() function per Jakub's request to
properly release DbgHelp resources via SymCleanup() on process exit.
This inadvertently was removed when the TAP tests were removed.

Tested with both debug and release builds. With PDB files present,
backtraces now show symbol names, offsets, addresses, filenames, and
line numbers. Without PDB files (release build), exported function names
are still resolved.

Bryan