On 10/30/26 00:08, Michael Paquier wrote:
On Thu, Oct 29, 2026 at 06:11:39PM +0000, Burd, Greg wrote:
Validation of c97931c16d on the same machine: with it applied, three
runs each of both reproductions give the WARNING and ERROR and no
crash, and t/010_pg_basebackup.pl passes all 168 tests with no
backend crash in the server logs. With it reverted, the same TAP run
logs the SIGSEGV. The test still reports PASS in that case because
it only checks that pg_basebackup fails.
Ah, thanks for the in-depth investigation. So the previous guess was
really right. Candles work.
Thank you, Greg! This is exactly the confirmation we needed.
I would encourage you to file a bug with gcc. I've found a few bugs and
the gcc folks are really easy to work with. Since you have a repro it
will be easy to produce the pre-processed output they want.
One bad thing -- even if they fix the bug, distros may choose not to
back patch it (so it may already be fixed). I had to maintain a patch
for a gcc bug on ppc64le for years before the affected distros EOL'd.
That bug limited where I could use const, which was pretty annoying.
I don't see a better fix on our side. A helper function with
the array would also work, but palloc0() is simpler, and both rely on
the same thing: no PG_TRY() frame larger than 4kB. In a core build
on this machine, the only other function with a frame that large that
calls sigsetjmp() is be_lo_export() (char buf[BUFSIZE]). Its code
after sigsetjmp() reloads the constant, so it is fine today, but only
by luck of register allocation. I haven't checked contrib or other
GCC versions or as mentioned earlier against their bug system.
Ugh. Even if the buffer itself is not used within the try/catch
block itself?
Apparently so -- the failure of the base backup cancel test showed that.
The buffer was never used and just the larger frame caused the problem.
Getting entirely rid of the TRY/CATCH would lead to a
much nicer result overall: we'd be able to keep the local buffer. Or
perhaps go to the simpler solution of a palloc/pfree? I cannot
believe that this extra allocation would matter for lo_export() calls.
palloc/pfree seems like the simpler solution.
If you are proved to be right, it sounds to me that we ought to
fix and backpatch that as well, especially if there is nothing we can
do on the Postgres side. I don't believe in luck and magic when it
comes to software programming.
I've attached a patch for be_lo_export(). Working around compiler bugs
is never fun but I'm not sure what choice we have.
Regards,
-David
diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 10cc05d5e4b..42da83b9706 100644
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -489,7 +489,7 @@ be_lo_export(PG_FUNCTION_ARGS)
text *filename = PG_GETARG_TEXT_PP(1);
int fd;
int nbytes;
- char buf[BUFSIZE];
+ char *buf;
char fnamebuf[MAXPGPATH];
LargeObjectDesc *lobj;
mode_t oumask;
@@ -525,6 +525,13 @@ be_lo_export(PG_FUNCTION_ARGS)
errmsg("could not create server file \"%s\":
%m",
fnamebuf)));
+ /*
+ * The buffer is allocated rather than declared on the stack. A frame
+ * larger than 4kB that contains a PG_TRY() can be miscompiled by GCC on
+ * SPARC.
+ */
+ buf = palloc(BUFSIZE);
+
/*
* read in from the inversion file and write to the filesystem
*/
@@ -540,6 +547,8 @@ be_lo_export(PG_FUNCTION_ARGS)
fnamebuf)));
}
+ pfree(buf);
+
if (CloseTransientFile(fd) != 0)
ereport(ERROR,
(errcode_for_file_access(),
