From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: jsonapi: scary new warnings with LTO enabled
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2074633.1744839761.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 16 Apr 2025 17:42:41 -0400
Message-ID: <2074634.1744839761@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2074634.1744839761%40sss.pgh.pa.us>
Precedence: bulk

I noticed some new warnings from buildfarm member chafer,
which I'm able to reproduce locally on a Fedora 41 box
by building with "meson setup build -Db_lto=3Dtrue":

ninja: Entering directory `build'
[1515/2472] Linking target src/interfaces/libpq/libpq.so.5.18
In function 'freeJsonLexContext',
    inlined from 'freeJsonLexContext' at ../src/common/jsonapi.c:688:1,
    inlined from 'handle_oauth_sasl_error' at ../src/interfaces/libpq/fe-a=
uth-oauth.c:547:2:
../src/common/jsonapi.c:723:17: warning: 'free' called on unallocated obje=
ct 'lex' [-Wfree-nonheap-object]
  723 |                 FREE(lex);
      |                 ^
../src/interfaces/libpq/fe-auth-oauth.c: In function 'handle_oauth_sasl_er=
ror':
../src/interfaces/libpq/fe-auth-oauth.c:479:24: note: declared here
  479 |         JsonLexContext lex =3D {0};
      |                        ^
[2407/2472] Linking target src/test/modules/test_json_parser/test_json_par=
ser_incremental_shlib
In function 'freeJsonLexContext',
    inlined from 'freeJsonLexContext' at ../src/common/jsonapi.c:688:1,
    inlined from 'main' at ../src/test/modules/test_json_parser/test_json_=
parser_incremental.c:198:2:
../src/common/jsonapi.c:723:17: warning: 'free' called on unallocated obje=
ct 'lex' [-Wfree-nonheap-object]
  723 |                 FREE(lex);
      |                 ^
../src/test/modules/test_json_parser/test_json_parser_incremental.c: In fu=
nction 'main':
../src/test/modules/test_json_parser/test_json_parser_incremental.c:87:24:=
 note: declared here
   87 |         JsonLexContext lex;
      |                        ^
[2426/2472] Linking target src/test/modules/test_json_parser/test_json_par=
ser_incremental
In function 'pg_free',
    inlined from 'pfree' at ../src/common/fe_memutils.c:135:2,
    inlined from 'freeJsonLexContext' at ../src/common/jsonapi.c:723:3,
    inlined from 'freeJsonLexContext' at ../src/common/jsonapi.c:688:1,
    inlined from 'main' at ../src/test/modules/test_json_parser/test_json_=
parser_incremental.c:198:2:
../src/common/fe_memutils.c:107:9: warning: 'free' called on unallocated o=
bject 'lex' [-Wfree-nonheap-object]
  107 |         free(ptr);
      |         ^
../src/test/modules/test_json_parser/test_json_parser_incremental.c: In fu=
nction 'main':
../src/test/modules/test_json_parser/test_json_parser_incremental.c:87:24:=
 note: declared here
   87 |         JsonLexContext lex;
      |                        ^

AFAICT there is no actual bug here: the FREE() call is reached only if
the JSONLEX_FREE_STRUCT flag is set, which it should not be for these
call sites.  But evidently the LTO optimizer is not quite smart enough
to realize that.

It seems fairly dangerous to ignore -Wfree-nonheap-object warnings.
I feel like we ought to move to prevent these somehow.  I'm not sure
how other than giving up on stack allocation of JsonLexContexts,
though, especially if we consider the jsonapi API frozen.  But seeing
that there are only three such call sites and none of them seem in the
least performance-critical, maybe we should just do that?

			regards, tom lane