Message-ID: <a9b5c843-951f-470d-93b3-05b9beb28ff2@gmail.com>
Date: Mon, 23 Feb 2026 13:07:23 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] Add Windows support for backtrace_functions (MSVC only)
To: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>
Cc: Euler Taveira <euler@eulerto.com>,
 Jakub Wartak <jakub.wartak@enterprisedb.com>,
 Michael Paquier <michael@paquier.xyz>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <202602091656.sxosgjxnc42l@alvherre.pgsql>
Content-Language: en-US
From: Bryan Green <dbryan.green@gmail.com>
In-Reply-To: <202602091656.sxosgjxnc42l@alvherre.pgsql>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/a9b5c843-951f-470d-93b3-05b9beb28ff2%40gmail.com>
Precedence: bulk

On 2/9/2026 11:17 AM, Álvaro Herrera wrote:
> 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.
> 
Hi Álvaro,

Thanks for the fixups — I've applied both patches and confirmed
everything works correctly on Windows.

You are correct to change the elog(WARNING) calls. Embedding the
diagnostic directly into the backtrace string is much cleaner, and your
approach of printing the address with a parenthetical note ([0x%llx]
(symbol lookup failed: error code %lu)) puts the information exactly
where it belongs, in context.

The backtrace_cleanup() restructuring also makes sense — making it
unconditionally defined with pg_attribute_unused() and putting the
#ifdef _MSC_VER guard inside the body is cleaner.

I confirmed it compiles and links successfully on Windows with MSVC. I
ran several tests and the backtrace feature is working as expected.

Thanks again to everyone who reviewed this.

-- 
Bryan Green
EDB: https://www.enterprisedb.com