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

From: "Euler Taveira" <euler(at)eulerto(dot)com>
To: "Bryan Green" <dbryan(dot)green(at)gmail(dot)com>, "Jakub Wartak" <jakub(dot)wartak(at)enterprisedb(dot)com>
Cc: Álvaro Herrera <alvherre(at)kurilemu(dot)de>, "Michael Paquier" <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
Date: 2025-11-01 15:31:15
Message-ID: cbca062e-98df-4f07-b8ae-f757c1ee89d7@app.fastmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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

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

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

--
Euler Taveira
EDB https://www.enterprisedb.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David E. Wheeler 2025-11-01 15:49:50 Re: abi-compliance-check failure due to recent changes to pg_{clear,restore}_{attribute,relation}_stats()
Previous Message Marcos Pegoraro 2025-11-01 14:33:24 Re: Missing parentheses