Skip to content

Commit d9754c5

Browse files
fanfuxiaoranhackorum
fanfuxiaoran
authored and
hackorum
committed
Imporve pg_re_throw: check if sigjmp_buf is valid and report error
If the code in PG_TRY contains any non local control flow other than ereport(ERROR) like goto, break etc., the PG_CATCH or PG_END_TRY cannot be called, then the PG_exception_stack will point to the memory whose stack frame has been released. So after that, when the pg_re_throw called, __longjmp() will crash and report Segmentation fault error. In that case, to help developers to figure out the root cause easily, it is better to report that 'the sigjmp_buf is invalid' rather than letting the __longjmp report any error. Addition to sigjmp_buf, add another field 'int magic' which is next to the sigjum_buf in the local stack frame memory. The magic's value is always 'PG_exception_magic 0x12345678'. And in 'pg_re_throw' routine, check if the magic's value is still '0x12345678', if not, that means the memory where the 'PG_exception_stack' points to has been released, and the 'sigbuf' must be invalid.
1 parent db0c984 commit d9754c5

11 files changed

Lines changed: 40 additions & 28 deletions

File tree

‎src/backend/postmaster/autovacuum.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ static void check_av_worker_gucs(void);
410410
void
411411
AutoVacLauncherMain(const void *startup_data, size_t startup_data_len)
412412
{
413-
sigjmp_buf local_sigjmp_buf;
413+
PG_exception local_sigjmp_buf = {PG_exception_magic};
414414

415415
Assert(startup_data_len == 0);
416416

@@ -484,7 +484,7 @@ AutoVacLauncherMain(const void *startup_data, size_t startup_data_len)
484484
* call redundant, but it is not since InterruptPending might be set
485485
* already.
486486
*/
487-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
487+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
488488
{
489489
/* since not using PG_TRY, must reset error stack by hand */
490490
error_context_stack = NULL;
@@ -1417,7 +1417,7 @@ avl_sigusr2_handler(SIGNAL_ARGS)
14171417
void
14181418
AutoVacWorkerMain(const void *startup_data, size_t startup_data_len)
14191419
{
1420-
sigjmp_buf local_sigjmp_buf;
1420+
PG_exception local_sigjmp_buf = {PG_exception_magic};
14211421
Oid dbid;
14221422

14231423
Assert(startup_data_len == 0);
@@ -1478,7 +1478,7 @@ AutoVacWorkerMain(const void *startup_data, size_t startup_data_len)
14781478
* seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
14791479
* it is not since InterruptPending might be set already.
14801480
*/
1481-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
1481+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
14821482
{
14831483
/* since not using PG_TRY, must reset error stack by hand */
14841484
error_context_stack = NULL;

‎src/backend/postmaster/bgworker.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ SanityCheckBackgroundWorker(BackgroundWorker *worker, int elevel)
740740
void
741741
BackgroundWorkerMain(const void *startup_data, size_t startup_data_len)
742742
{
743-
sigjmp_buf local_sigjmp_buf;
743+
PG_exception local_sigjmp_buf = {PG_exception_magic};
744744
BackgroundWorker *worker;
745745
bgworker_main_type entrypt;
746746

@@ -804,7 +804,7 @@ BackgroundWorkerMain(const void *startup_data, size_t startup_data_len)
804804
*
805805
* We just need to clean up, report the error, and go away.
806806
*/
807-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
807+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
808808
{
809809
/* Since not using PG_TRY, must reset error stack by hand */
810810
error_context_stack = NULL;

‎src/backend/postmaster/bgwriter.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ static XLogRecPtr last_snapshot_lsn = InvalidXLogRecPtr;
8888
void
8989
BackgroundWriterMain(const void *startup_data, size_t startup_data_len)
9090
{
91-
sigjmp_buf local_sigjmp_buf;
91+
PG_exception local_sigjmp_buf = {PG_exception_magic};
9292
MemoryContext bgwriter_context;
9393
bool prev_hibernate;
9494
WritebackContext wb_context;
@@ -151,7 +151,7 @@ BackgroundWriterMain(const void *startup_data, size_t startup_data_len)
151151
* call redundant, but it is not since InterruptPending might be set
152152
* already.
153153
*/
154-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
154+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
155155
{
156156
/* Since not using PG_TRY, must reset error stack by hand */
157157
error_context_stack = NULL;

‎src/backend/postmaster/checkpointer.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ static void ReqShutdownXLOG(SIGNAL_ARGS);
205205
void
206206
CheckpointerMain(const void *startup_data, size_t startup_data_len)
207207
{
208-
sigjmp_buf local_sigjmp_buf;
208+
PG_exception local_sigjmp_buf = {PG_exception_magic};
209209
MemoryContext checkpointer_context;
210210

211211
Assert(startup_data_len == 0);
@@ -283,7 +283,7 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len)
283283
* call redundant, but it is not since InterruptPending might be set
284284
* already.
285285
*/
286-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
286+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
287287
{
288288
/* Since not using PG_TRY, must reset error stack by hand */
289289
error_context_stack = NULL;
@@ -347,7 +347,6 @@ CheckpointerMain(const void *startup_data, size_t startup_data_len)
347347

348348
/* We can now handle ereport(ERROR) */
349349
PG_exception_stack = &local_sigjmp_buf;
350-
351350
/*
352351
* Unblock signals (they were blocked when the postmaster forked us)
353352
*/

‎src/backend/postmaster/pgarch.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ pgarch_ArchiverCopyLoop(void)
519519
static bool
520520
pgarch_archiveXlog(char *xlog)
521521
{
522-
sigjmp_buf local_sigjmp_buf;
522+
PG_exception local_sigjmp_buf = {PG_exception_magic};
523523
MemoryContext oldcontext;
524524
char pathname[MAXPGPATH];
525525
char activitymsg[MAXFNAMELEN + 16];
@@ -548,7 +548,7 @@ pgarch_archiveXlog(char *xlog)
548548
* PgArchiverMain() and use PG_TRY/PG_CATCH here, but the extra code to
549549
* avoid the odd archiver restart doesn't seem worth it.
550550
*/
551-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
551+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
552552
{
553553
/* Since not using PG_TRY, must reset error stack by hand */
554554
error_context_stack = NULL;

‎src/backend/postmaster/walsummarizer.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ WalSummarizerShmemInit(void *arg)
222222
void
223223
WalSummarizerMain(const void *startup_data, size_t startup_data_len)
224224
{
225-
sigjmp_buf local_sigjmp_buf;
225+
PG_exception local_sigjmp_buf = {PG_exception_magic};
226226
MemoryContext context;
227227

228228
/*
@@ -284,7 +284,7 @@ WalSummarizerMain(const void *startup_data, size_t startup_data_len)
284284
/*
285285
* If an exception is encountered, processing resumes here.
286286
*/
287-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
287+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
288288
{
289289
/* Since not using PG_TRY, must reset error stack by hand */
290290
error_context_stack = NULL;

‎src/backend/postmaster/walwriter.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ int WalWriterFlushAfter = DEFAULT_WAL_WRITER_FLUSH_AFTER;
8888
void
8989
WalWriterMain(const void *startup_data, size_t startup_data_len)
9090
{
91-
sigjmp_buf local_sigjmp_buf;
91+
PG_exception local_sigjmp_buf = {PG_exception_magic};
9292
MemoryContext walwriter_context;
9393
int left_till_hibernate;
9494
bool hibernating;
@@ -143,7 +143,7 @@ WalWriterMain(const void *startup_data, size_t startup_data_len)
143143
* call redundant, but it is not since InterruptPending might be set
144144
* already.
145145
*/
146-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
146+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
147147
{
148148
/* Since not using PG_TRY, must reset error stack by hand */
149149
error_context_stack = NULL;

‎src/backend/replication/logical/slotsync.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,7 +1609,7 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len)
16091609
WalReceiverConn *wrconn = NULL;
16101610
char *dbname;
16111611
char *err;
1612-
sigjmp_buf local_sigjmp_buf;
1612+
PG_exception local_sigjmp_buf = {PG_exception_magic};
16131613
StringInfoData app_name;
16141614

16151615
Assert(startup_data_len == 0);
@@ -1647,7 +1647,7 @@ ReplSlotSyncWorkerMain(const void *startup_data, size_t startup_data_len)
16471647
* operates at the bottom of the exception stack, ERRORs turn into FATALs.
16481648
* Therefore, we create our own exception handler to catch ERRORs.
16491649
*/
1650-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
1650+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
16511651
{
16521652
/* since not using PG_TRY, must reset error stack by hand */
16531653
error_context_stack = NULL;

‎src/backend/tcop/postgres.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4363,7 +4363,7 @@ PostgresSingleUserMain(int argc, char *argv[],
43634363
void
43644364
PostgresMain(const char *dbname, const char *username)
43654365
{
4366-
sigjmp_buf local_sigjmp_buf;
4366+
PG_exception local_sigjmp_buf = {PG_exception_magic};
43674367

43684368
/* these must be volatile to ensure state is preserved across longjmp: */
43694369
volatile bool send_ready_for_query = true;
@@ -4570,7 +4570,7 @@ PostgresMain(const char *dbname, const char *username)
45704570
* were inside a transaction.
45714571
*/
45724572

4573-
if (sigsetjmp(local_sigjmp_buf, 1) != 0)
4573+
if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
45744574
{
45754575
/*
45764576
* NOTE: if you are tempted to add more code in this if-block,
@@ -4685,7 +4685,6 @@ PostgresMain(const char *dbname, const char *username)
46854685

46864686
/* We can now handle ereport(ERROR) */
46874687
PG_exception_stack = &local_sigjmp_buf;
4688-
46894688
if (!ignore_till_sync)
46904689
send_ready_for_query = true; /* initially, or after error */
46914690

‎src/backend/utils/error/elog.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@
9999
/* Global variables */
100100
ErrorContextCallback *error_context_stack = NULL;
101101

102-
sigjmp_buf *PG_exception_stack = NULL;
102+
PG_exception *PG_exception_stack = NULL;
103103

104104
/*
105105
* Hook for intercepting messages before they are sent to the server log.
@@ -2201,7 +2201,15 @@ pg_re_throw(void)
22012201
{
22022202
/* If possible, throw the error to the next outer setjmp handler */
22032203
if (PG_exception_stack != NULL)
2204-
siglongjmp(*PG_exception_stack, 1);
2204+
{
2205+
#pragma clang diagnostic push
2206+
#pragma clang diagnostic ignored "-Wunused"
2207+
void *stackTop = NULL;
2208+
#pragma clang diagnostic pop
2209+
Assert((void *)PG_exception_stack > &stackTop ||
2210+
PG_exception_stack->magic == PG_exception_magic);
2211+
siglongjmp(PG_exception_stack->buf, 1);
2212+
}
22052213
else
22062214
{
22072215
/*

0 commit comments

Comments
 (0)