From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
cc: Andres Freund <andres@anarazel.de>, Xing Guo <higuoxing@gmail.com>,
        pgsql-hackers@postgresql.org
Subject: Re: PL/Python: Fix return in the middle of PG_TRY() block.
In-reply-to: <20230503202116.GA2110623@nathanxps13>
References: <CACpMh+CMsGMRKFzFMm3bYTzQmMU5nfEEoEDU2apJcc4hid36AQ@mail.gmail.com> <20230112184433.GA2104952@nathanxps13> <20230113054900.b7onkvwtkrykeu3z@awork3.anarazel.de> <20230113180335.GA2160040@nathanxps13> <CACpMh+D9jz4Lq3qieoU2NkhGASqekhDXnXX2USW4mHa+SWNQag@mail.gmail.com> <1864041.1673882943@sss.pgh.pa.us> <20230120010711.jpqbaelpgmuvt2vq@awork3.anarazel.de> <20230120190201.GC4106863@nathanxps13> <20230503202116.GA2110623@nathanxps13>
Comments: In-reply-to Nathan Bossart <nathandbossart@gmail.com>
	message dated "Wed, 03 May 2023 13:21:16 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3012765.1683146012.1@sss.pgh.pa.us>
Date: Wed, 03 May 2023 16:33:32 -0400
Message-ID: <3012766.1683146012@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/3012766.1683146012%40sss.pgh.pa.us>
Precedence: bulk

Nathan Bossart <nathandbossart@gmail.com> writes:
> Here's a new version of the patch.  Besides adding comments and a commit
> message, I made sure to decrement the reference count for pltargs in the
> PG_CATCH block (which means that pltargs likely needs to be volatile).

Hmm, actually I think these changes should allow you to *remove* some
volatile markers.  IIUC, you need volatile for variables that are declared
outside PG_TRY but modified within it.  That is the case for these
pointers as the code stands, but your patch is changing them to the
non-risky case where they are assigned once before entering PG_TRY.

(My mental model of this is that without "volatile", the compiler
may keep the variable in a register, creating the hazard that longjmp
will revert the variable's value to what it was at setjmp time thanks
to the register save/restore that those functions do.  But if it hasn't
changed value since entering PG_TRY, then that doesn't matter.)

> I'm
> not too wild about moving the chunk of code for pltargs like this, but I
> haven't thought of a better option.  We could error instead of returning
> NULL, but IIUC that would go against d0aa965's stated purpose.

Agreed, throwing an error in these situations doesn't improve matters.

			regards, tom lane