On Fri, Jun 28, 2024 at 12:14 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > Avoid crashing when a JIT-inlined backend function throws an error. > > errfinish() assumes that the __FUNC__ and __FILE__ arguments it's > passed are compile-time constant strings that can just be pointed > to rather than physically copied. However, it's possible for LLVM > to generate code in which those pointers point into a dynamically > loaded code segment. If that segment gets unloaded before we're > done with the ErrorData struct, we have dangling pointers that > will lead to SIGSEGV. In simple cases that won't happen, because we > won't unload LLVM code before end of transaction. But it's possible > to happen if the error is thrown within end-of-transaction code run by > _SPI_commit or _SPI_rollback, because since commit 2e517818f those > functions clean up by ending the transaction and starting a new one. > > Rather than fixing this by adding pstrdup() overhead to every > elog/ereport sequence, let's fix it by copying the risky pointers > in CopyErrorData(). That solves it for _SPI_commit/_SPI_rollback > because they use that function to preserve the error data across > the transaction end/restart sequence; and it seems likely that > any other code doing something similar would need to do that too. > > I'm suspicious that this behavior amounts to an LLVM bug (or a > bug in our use of it?), because it implies that string constant > references that should be pointer-equal according to a naive > understanding of C semantics will sometimes not be equal. > However, even if it is a bug and someday gets fixed, we'll have > to cope with the current behavior for a long time to come. > > Report and patch by me. Back-patch to all supported branches. > > Discussion: https://postgr.es/m/1565654.1719425...@sss.pgh.pa.us > > Branch > ------ > REL_14_STABLE > > Details > ------- > https://git.postgresql.org/pg/commitdiff/13abc1f660740edab9c32d1ad4ca0fb5d5f387f6 > > Modified Files > -------------- > src/backend/utils/error/elog.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) >
Hi Tom, Should we make a similar change to ReThrowError() as well? I'm particularly concerned about cases where someone might copy error data using CopyErrorData() and then rethrowing that copied edata. Regards, Amul
