public inbox for [email protected]help / color / mirror / Atom feed
Re: null iv parameter passed to combo_init() 6+ messages / 4 participants [nested] [flat]
* Re: null iv parameter passed to combo_init() @ 2022-01-09 12:37 Zhihong Yu <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Zhihong Yu @ 2022-01-09 12:37 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Noah Misch <[email protected]>; pgsql-hackers On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <[email protected]> wrote: > Noah Misch <[email protected]> writes: > > On further thought, I would write it this way: > > > - else > > + else if (ivlen != 0) > > memcpy(ivbuf, iv, ivlen); > > FWIW, I liked the "ivlen > 0" formulation better. They should be > equivalent, because ivlen is unsigned, but it just seems like "> 0" > is more natural. > > regards, tom lane > Patch v4 is attached. Cheers Attachments: [application/octet-stream] 0004-memcpy-null.patch (2.2K, ../../CALNJ-vTuaO+6hM4wY2aE0V7WdB56bwTXDp50Bn2U1ggmonVGbA@mail.gmail.com/3-0004-memcpy-null.patch) download | inline diff: diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c index 4205e9c3ef..2099ce2a7a 100644 --- a/contrib/pgcrypto/px.c +++ b/contrib/pgcrypto/px.c @@ -200,7 +200,7 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen, ivbuf = palloc0(ivs); if (ivlen > ivs) memcpy(ivbuf, iv, ivs); - else + else if (ivlen > 0) memcpy(ivbuf, iv, ivlen); } diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index de787c3d37..eca1e5dd0e 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -294,7 +294,7 @@ TransactionIdSetPageStatus(TransactionId xid, int nsubxids, * sub-XIDs and all of the XIDs for which we're adjusting clog should be * on the same page. Check those conditions, too. */ - if (all_xact_same_page && xid == MyProc->xid && + if (subxids != NULL && all_xact_same_page && xid == MyProc->xid && nsubxids <= THRESHOLD_SUBTRANS_CLOG_OPT && nsubxids == MyProc->subxidStatus.count && memcmp(subxids, MyProc->subxids.xids, diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index c9516e03fa..3d5ca89d42 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -5329,7 +5329,7 @@ SerializeTransactionState(Size maxsize, char *start_address) * of our own, we can just pass along the information that was passed to * us. */ - if (nParallelCurrentXids > 0) + if (nParallelCurrentXids > 0 && ParallelCurrentXids != NULL) { result->nParallelCurrentXids = nParallelCurrentXids; memcpy(&result->parallelCurrentXids[0], ParallelCurrentXids, diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index e5cf1bde13..ae10c55711 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1182,7 +1182,10 @@ DefineIndex(Oid relationId, pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL, nparts); - memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts); + if (partdesc->oids != NULL) + { + memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts); + } parentDesc = RelationGetDescr(rel); opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: null iv parameter passed to combo_init() @ 2022-01-09 16:48 Noah Misch <[email protected]> parent: Zhihong Yu <[email protected]> 0 siblings, 2 replies; 6+ messages in thread From: Noah Misch @ 2022-01-09 16:48 UTC (permalink / raw) To: Tom Lane <[email protected]>; Zhihong Yu <[email protected]>; +Cc: pgsql-hackers On Sun, Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote: > On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <[email protected]> wrote: > > Noah Misch <[email protected]> writes: > > > On further thought, I would write it this way: > > > > > - else > > > + else if (ivlen != 0) > > > memcpy(ivbuf, iv, ivlen); > > > > FWIW, I liked the "ivlen > 0" formulation better. They should be > > equivalent, because ivlen is unsigned, but it just seems like "> 0" > > is more natural. If I were considering the one code site in isolation, I'd pick "ivlen > 0". But of the four sites identified so far, three have signed length variables. Since we're likely to get more examples of this pattern, some signed and some unsigned, I'd rather use a style that does the optimal thing whether or not the variable is signed. What do you think? > Patch v4 is attached. Does this pass the test procedure shown upthread? ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: null iv parameter passed to combo_init() @ 2022-01-09 16:51 Tom Lane <[email protected]> parent: Noah Misch <[email protected]> 1 sibling, 0 replies; 6+ messages in thread From: Tom Lane @ 2022-01-09 16:51 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: Zhihong Yu <[email protected]>; pgsql-hackers Noah Misch <[email protected]> writes: > On Sun, Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote: >> On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <[email protected]> wrote: >>> FWIW, I liked the "ivlen > 0" formulation better. They should be >>> equivalent, because ivlen is unsigned, but it just seems like "> 0" >>> is more natural. > If I were considering the one code site in isolation, I'd pick "ivlen > 0". > But of the four sites identified so far, three have signed length variables. Oh, hmm. Unless we want to start changing those to unsigned, I agree a not-equal test is a safer convention. regards, tom lane ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: null iv parameter passed to combo_init() @ 2022-01-09 20:38 Zhihong Yu <[email protected]> parent: Noah Misch <[email protected]> 1 sibling, 1 reply; 6+ messages in thread From: Zhihong Yu @ 2022-01-09 20:38 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers On Sun, Jan 9, 2022 at 8:48 AM Noah Misch <[email protected]> wrote: > On Sun, Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote: > > On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <[email protected]> wrote: > > > Noah Misch <[email protected]> writes: > > > > On further thought, I would write it this way: > > > > > > > - else > > > > + else if (ivlen != 0) > > > > memcpy(ivbuf, iv, ivlen); > > > > > > FWIW, I liked the "ivlen > 0" formulation better. They should be > > > equivalent, because ivlen is unsigned, but it just seems like "> 0" > > > is more natural. > > If I were considering the one code site in isolation, I'd pick "ivlen > 0". > But of the four sites identified so far, three have signed length > variables. > Since we're likely to get more examples of this pattern, some signed and > some > unsigned, I'd rather use a style that does the optimal thing whether or not > the variable is signed. What do you think? > > > Patch v4 is attached. > > Does this pass the test procedure shown upthread? > Hi, I installed gcc 4.9.3 When I ran: ./configure CFLAGS='-fsanitize=undefined -fsanitize-undefined-trap-on-error' I saw: configure:3977: $? = 0 configure:3966: gcc -V >&5 gcc: error: unrecognized command line option '-V' gcc: fatal error: no input files compilation terminated. configure:3977: $? = 1 configure:3966: gcc -qversion >&5 gcc: error: unrecognized command line option '-qversion' gcc: fatal error: no input files compilation terminated. configure:3977: $? = 1 configure:3997: checking whether the C compiler works configure:4019: gcc -fsanitize=undefined -fsanitize-undefined-trap-on-error conftest.c >&5 gcc: error: unrecognized command line option '-fsanitize-undefined-trap-on-error' configure:4023: $? = 1 configure:4061: result: no I wonder if a higher version gcc is needed. FYI ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: null iv parameter passed to combo_init() @ 2022-01-09 21:27 Zhihong Yu <[email protected]> parent: Zhihong Yu <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Zhihong Yu @ 2022-01-09 21:27 UTC (permalink / raw) To: Noah Misch <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers On Sun, Jan 9, 2022 at 12:38 PM Zhihong Yu <[email protected]> wrote: > > > On Sun, Jan 9, 2022 at 8:48 AM Noah Misch <[email protected]> wrote: > >> On Sun, Jan 09, 2022 at 04:37:32AM -0800, Zhihong Yu wrote: >> > On Sat, Jan 8, 2022 at 11:32 PM Tom Lane <[email protected]> wrote: >> > > Noah Misch <[email protected]> writes: >> > > > On further thought, I would write it this way: >> > > >> > > > - else >> > > > + else if (ivlen != 0) >> > > > memcpy(ivbuf, iv, ivlen); >> > > >> > > FWIW, I liked the "ivlen > 0" formulation better. They should be >> > > equivalent, because ivlen is unsigned, but it just seems like "> 0" >> > > is more natural. >> >> If I were considering the one code site in isolation, I'd pick "ivlen > >> 0". >> But of the four sites identified so far, three have signed length >> variables. >> Since we're likely to get more examples of this pattern, some signed and >> some >> unsigned, I'd rather use a style that does the optimal thing whether or >> not >> the variable is signed. What do you think? >> >> > Patch v4 is attached. >> >> Does this pass the test procedure shown upthread? >> > Hi, > I installed gcc 4.9.3 > > When I ran: > ./configure CFLAGS='-fsanitize=undefined > -fsanitize-undefined-trap-on-error' > > I saw: > > configure:3977: $? = 0 > configure:3966: gcc -V >&5 > gcc: error: unrecognized command line option '-V' > gcc: fatal error: no input files > compilation terminated. > configure:3977: $? = 1 > configure:3966: gcc -qversion >&5 > gcc: error: unrecognized command line option '-qversion' > gcc: fatal error: no input files > compilation terminated. > configure:3977: $? = 1 > configure:3997: checking whether the C compiler works > configure:4019: gcc -fsanitize=undefined > -fsanitize-undefined-trap-on-error conftest.c >&5 > gcc: error: unrecognized command line option > '-fsanitize-undefined-trap-on-error' > configure:4023: $? = 1 > configure:4061: result: no > > I wonder if a higher version gcc is needed. > > FYI > After installing gcc-11, ./configure passed (with 0003-memcpy-null.patch). In the output of `make check-world`, I don't see `runtime error`. Though there was a crash (maybe specific to my machine): Core was generated by `/nfusr/dev-server/zyu/postgres/tmp_install/usr/local/pgsql/bin/postgres --singl'. Program terminated with signal SIGILL, Illegal instruction. #0 0x000000000050642d in write_item.cold () Missing separate debuginfos, use: debuginfo-install glibc-2.17-325.el7_9.x86_64 nss-pam-ldapd-0.8.13-25.el7.x86_64 sssd-client-1.16.5-10.el7_9.10.x86_64 (gdb) bt #0 0x000000000050642d in write_item.cold () #1 0x0000000000ba9d1b in write_relcache_init_file () #2 0x0000000000bb58f7 in RelationCacheInitializePhase3 () #3 0x0000000000bd5cb5 in InitPostgres () #4 0x0000000000a0a9ea in PostgresMain () FYI ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH vREL_10_STABLE 1/2] Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin(). @ 2022-04-29 19:50 Andres Freund <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Andres Freund @ 2022-04-29 19:50 UTC (permalink / raw) Author: Reviewed-By: Discussion: https://postgr.es/m/[email protected] Backpatch: --- src/backend/storage/ipc/standby.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index 863cb641007..47cdaead9e6 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -44,6 +44,7 @@ static HTAB *RecoveryLockLists; /* Flags set by timeout handlers */ static volatile sig_atomic_t got_standby_deadlock_timeout = false; +static volatile sig_atomic_t got_standby_delay_timeout = false; static volatile sig_atomic_t got_standby_lock_timeout = false; static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist, @@ -593,10 +594,15 @@ ResolveRecoveryConflictWithBufferPin(void) enable_timeouts(timeouts, cnt); } - /* Wait to be signaled by UnpinBuffer() */ + /* + * Wait to be signaled by UnpinBuffer() or for the wait to be interrupted + * by one of the timeouts established above. + */ ProcWaitForSignal(PG_WAIT_BUFFER_PIN); - if (got_standby_deadlock_timeout) + if (got_standby_delay_timeout) + SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN); + else if (got_standby_deadlock_timeout) { /* * Send out a request for hot-standby backends to check themselves for @@ -622,6 +628,7 @@ ResolveRecoveryConflictWithBufferPin(void) * individually, but that'd be slower. */ disable_all_timeouts(false); + got_standby_delay_timeout = false; got_standby_deadlock_timeout = false; } @@ -681,8 +688,8 @@ CheckRecoveryConflictDeadlock(void) */ /* - * StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT - * occurs before STANDBY_TIMEOUT. + * StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT is + * exceeded. */ void StandbyDeadLockHandler(void) @@ -692,16 +699,11 @@ StandbyDeadLockHandler(void) /* * StandbyTimeoutHandler() will be called if STANDBY_TIMEOUT is exceeded. - * Send out a request to release conflicting buffer pins unconditionally, - * so we can press ahead with applying changes in recovery. */ void StandbyTimeoutHandler(void) { - /* forget any pending STANDBY_DEADLOCK_TIMEOUT request */ - disable_timeout(STANDBY_DEADLOCK_TIMEOUT, false); - - SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN); + got_standby_delay_timeout = true; } /* -- 2.35.1.677.gabf474a5dd --ijiwrbjbttsgn7hm Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="vREL_10_STABLE-0002-Backpatch-031_recovery_conflict.pl.patch" ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2022-04-29 19:50 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-01-09 12:37 Re: null iv parameter passed to combo_init() Zhihong Yu <[email protected]> 2022-01-09 16:48 ` Noah Misch <[email protected]> 2022-01-09 16:51 ` Tom Lane <[email protected]> 2022-01-09 20:38 ` Zhihong Yu <[email protected]> 2022-01-09 21:27 ` Zhihong Yu <[email protected]> 2022-04-29 19:50 [PATCH vREL_10_STABLE 1/2] Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin(). Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox