Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 5 Jun 2024 14:04:39 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: ranier.vf@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: Fix use of possible uninitialized variable retval
 (src/pl/plpgsql/src/pl_handler.c)
Message-ID: <Zl_x53iNtSGoa3ms@paquier.xyz>
References: <CAEudQAoDXrVNV8M9q2k4xycxuBrebh0NRPECXFa1N-Z8q-B7ww@mail.gmail.com>
 <20240605.131241.1337361574087593814.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="xO9NpcoYGZ5VfYnV"
Content-Disposition: inline
In-Reply-To: <20240605.131241.1337361574087593814.horikyota.ntt@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Zl_x53iNtSGoa3ms%40paquier.xyz>
Precedence: bulk


--xO9NpcoYGZ5VfYnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 05, 2024 at 01:12:41PM +0900, Kyotaro Horiguchi wrote:
> At Mon, 27 May 2024 11:31:24 -0300, Ranier Vilela <ranier.vf@gmail.com> w=
rote in=20
>> The function *plpgsql_inline_handler* can use uninitialized
>> variable retval, if PG_TRY fails.
>> Fix like function*plpgsql_call_handler* wich declare retval as
>> volatile and initialize to (Datum 0).

You forgot to read elog.h, explaining under which circumstances
variables related to TRY/CATCH block should be marked as volatile.
There is a big "Note:" paragraph.

It is not the first time that this is mentioned on this list: but
sending a report without looking at the reason why a change is
justified makes everybody waste time.  That's not productive.

> If PG_TRY fails, retval is not actually accessed, so no real issue
> exists. Commit 7292fd8f1c changed plpgsql_call_handler() to the
> current form, but as stated in its commit message, it did not fix a
> real issue and was solely to silence compiler.

This complain was from lapwing, that uses a version of gcc which
produces a lot of noise with incorrect issues.  It is one of the only
32b buildfarm members, so it still has a lot of value.

> I believe we do not need to modify plpgsql_inline_handler() unless
> compiler actually issues a false warning for it.

If we were to do something, that would be to remove the volatile from
plpgsql_call_handler() at the end once we don't have in the buildfarm
compilers that complain about it, because there is no reason to use a
volatile in this case.  :)
--
Michael

--xO9NpcoYGZ5VfYnV
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmZf8ecACgkQnvQgOdby
QH0T6Q/+LGeaYtBzOTlCNEoCpLGACXnu3HpU7tAf62cJ6l5eWhi7ti89ElyvWEs6
uZTFk5zBsxpxvEvOFBmEF2S123LiBqxk5V9Sp/JWgbl8/zL+2GTsuAo2N4S9htLZ
TTrjQxm2uDTjjAtKGVUgYlGv37AA8XTULZ5LFL5XYKlA3Ak08MdbktR253qdEJeH
ul/1nPO2NeSVmFRKmt6ua5omLNcjqxjCM5R0OLt5lh59JIAc3E7bkOOTkXqeDRrA
kvhKnI9RFUuyfemGjCd8HObbFm1sU5bYdLTmwmILdLAqeQKVs6Nk7tQPcplNcPBZ
mIHPzG9B3xBGk7fRbI5vs9FIKFFx7CrRs1MXK5rfGwwt3WxeZBp7AVLQvGDlhz+R
N8y3ptAz/SLNrTtfC+MMwuMTkH3YkulcPvQkf7tB8cixRwawB+niiHw3veK8YEIY
zbkYKEg29eAuOsSLvWz0Ks4HQ6OYgCFMUj8JgQQdruw1LAbMPHiwQ2b+cZiaGq3b
N1Z/1WSOnWXSuU1DsJtfQLRf4USIKONEMX2OdILg0+nEuToCEsMpnwH/+QVwFJO0
6rE06npO8G6AOOPJQx1cbxcSGRoNlNJh7t3YAVThWMoXRMk19JAH+Tu+fTf8EBKv
8TDFI6eYDYcEL3h98qXQJUKjJeFY7wZMLv3zW7woLPuLuBZnZxQ=
=HdOE
-----END PGP SIGNATURE-----

--xO9NpcoYGZ5VfYnV--