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
