| From: | Greg Burd <greg(at)burd(dot)me> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Bryan Green <dbryan(dot)green(at)gmail(dot)com>, Euler Taveira <euler(at)eulerto(dot)com>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, 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: | 2026-02-09 18:11:59 |
| Message-ID: | 303C7556-1877-4F4F-82E1-5DD05ADFFB08@greg.burd.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Feb 9 2026, at 12:17 pm, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
> On 2025-Nov-01, Bryan Green wrote:
>
Hey Bryan,
Thanks for working on this. I'm not a "Windows person" but I keep a
build-farm animal (unicorn - Win11/ARM64/MSVC) running, or at least I
try. :)
I saw this patch and felt it was a good idea, so I took a look at it
applied what Álvaro just sent out to my working tree on unicorn. I'm
fighting a few fires with it so having better diagnostic information
might help.
>> 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.
Hey Álvaro, I agree with this. Good catch.
> 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.
>
> --
> Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
I'm running tests again now, I'll let you know if the patch helped me to
identify the issue I'm having.
best.
-greg
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Álvaro Herrera | 2026-02-09 18:23:52 | Re: Remove HeapTupleheaderSetXmin{Committed,Invalid} functions |
| Previous Message | Dean Rasheed | 2026-02-09 18:09:28 | Re: ON CONFLICT DO SELECT (take 3) |