From: Daniel Gustafsson <daniel@yesql.se>
Message-Id: <0C92C876-09A3-483C-82C5-E26F0C1B34B5@yesql.se>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_0BAD372B-A8CE-4356-B2C0-344EA112A4D5"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3776.700.51.11.1\))
Subject: Re: jsonapi: scary new warnings with LTO enabled
Date: Tue, 22 Apr 2025 12:10:29 +0200
In-Reply-To: <CAOYmi+n7eShndhpJAZ-6+9h012nZfmuQbnj-p-L=ZnX96-PGVA@mail.gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
To: Jacob Champion <jacob.champion@enterprisedb.com>
References: <2074634.1744839761@sss.pgh.pa.us>
 <0A3CF5C4-9947-42E3-89A9-34FCF7BA95BA@yesql.se>
 <2078511.1744841564@sss.pgh.pa.us>
 <DCD56158-A642-4954-87FF-9DAD5425F3FF@yesql.se>
 <2084687.1744844690@sss.pgh.pa.us>
 <CAOYmi+n+DHpaEV5hymhufutTgc+S=+pDQfXEUi4hMAV1eO8aTw@mail.gmail.com>
 <2087556.1744846124@sss.pgh.pa.us>
 <0A596D8B-24F3-408F-B89B-B42014B00A05@yesql.se>
 <2202326.1744903233@sss.pgh.pa.us>
 <CAOYmi+mxjW0GhSGPKoOy3TYrFcdTM=5FcUn=u1i7CFhb2303Zg@mail.gmail.com>
 <25801148-284F-41F9-A279-A4CF92623C77@yesql.se>
 <CAOYmi+=w3Q-3F=LCr9hPRhiG2HGv6sogGOJW3v9pwpoCD6wN+w@mail.gmail.com>
 <E4E01D53-5E8C-4D93-85A8-633E93D0A194@yesql.se>
 <CAOYmi+k+Cv4PrD9HWRh2gzZLweKK+375hBV0-N4Q9Zzb5PM24w@mail.gmail.com>
 <102E1997-FB99-4091-926C-9A8BB3EA168E@yesql.se>
 <CAOYmi+n7eShndhpJAZ-6+9h012nZfmuQbnj-p-L=ZnX96-PGVA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/0C92C876-09A3-483C-82C5-E26F0C1B34B5%40yesql.se>
Precedence: bulk


--Apple-Mail=_0BAD372B-A8CE-4356-B2C0-344EA112A4D5
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

> On 21 Apr 2025, at 20:58, Jacob Champion =
<jacob.champion@enterprisedb.com> wrote:

> Personally, I'm fine with can't-fail APIs, as long as they're
> documented as such. (I think the deferred error API was probably
> chosen so that src/common JSON clients could be written without a lot
> of pain?)

My preference is that no operation can silently work on a failed object, =
but
it's not a hill (even more so given where we are in the cycle).  The =
attached
v3 allocates via the JSON api, no specific error handling should be =
required as
it's already handled today.

--
Daniel Gustafsson


--Apple-Mail=_0BAD372B-A8CE-4356-B2C0-344EA112A4D5
Content-Disposition: attachment;
	filename=v3-0001-Allocate-JsonLexContexts-on-the-heap-to-avoid-war.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v3-0001-Allocate-JsonLexContexts-on-the-heap-to-avoid-war.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20fe421d657aaa361dfb796e28c96975bed97f79f5=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Thu,=2017=20Apr=202025=2023:18:58=20=
+0200=0ASubject:=20[PATCH=20v3]=20Allocate=20JsonLexContexts=20on=20the=20=
heap=20to=20avoid=20warnings=0A=0AThe=20stack=20allocated=20=
JsonLexContexts,=20in=20combination=20with=20codepaths=0Ausing=20goto,=20=
were=20causing=20warnings=20when=20compiling=20with=20LTO=20enabled=0Aas=20=
the=20optimizer=20is=20unable=20to=20figure=20out=20that=20is=20safe.=20=20=
Rather=20than=0Acontort=20the=20code=20with=20workarounds=20for=20this=20=
simply=20heap=20allocate=20the=0Astructs=20instead=20as=20these=20are=20=
not=20in=20any=20performance=20critical=20paths.=0A=0AReported-by:=20Tom=20=
Lane=20<tgl@sss.pgh.pa.us>=0AReviewed-by:=20Jacob=20Champion=20=
<jacob.champion@enterprisedb.com>=0AReviewed-by:=20Tom=20Lane=20=
<tgl@sss.pgh.pa.us>=0ADiscussion:=20=
https://postgr.es/m/2074634.1744839761@sss.pgh.pa.us=0A---=0A=20=
src/interfaces/libpq/fe-auth-oauth.c=20=20=20=20=20=20=20=20=20=20|=2012=20=
+++++-----=0A=20.../test_json_parser_incremental.c=20=20=20=20=20=20=20=20=
=20=20=20=20|=2023=20+++++++++++--------=0A=202=20files=20changed,=2020=20=
insertions(+),=2015=20deletions(-)=0A=0Adiff=20--git=20=
a/src/interfaces/libpq/fe-auth-oauth.c=20=
b/src/interfaces/libpq/fe-auth-oauth.c=0Aindex=20=
cf1a25e2ccc..52d2b286625=20100644=0A---=20=
a/src/interfaces/libpq/fe-auth-oauth.c=0A+++=20=
b/src/interfaces/libpq/fe-auth-oauth.c=0A@@=20-476,7=20+476,7=20@@=20=
issuer_from_well_known_uri(PGconn=20*conn,=20const=20char=20*wkuri)=0A=20=
static=20bool=0A=20handle_oauth_sasl_error(PGconn=20*conn,=20const=20=
char=20*msg,=20int=20msglen)=0A=20{=0A-=09JsonLexContext=20lex=20=3D=20=
{0};=0A+=09JsonLexContext=20*lex;=0A=20=09JsonSemAction=20sem=20=3D=20=
{0};=0A=20=09JsonParseErrorType=20err;=0A=20=09struct=20json_ctx=20ctx=20=
=3D=20{0};=0A@@=20-504,8=20+504,8=20@@=20handle_oauth_sasl_error(PGconn=20=
*conn,=20const=20char=20*msg,=20int=20msglen)=0A=20=09=09return=20false;=0A=
=20=09}=0A=20=0A-=09makeJsonLexContextCstringLen(&lex,=20msg,=20msglen,=20=
PG_UTF8,=20true);=0A-=09setJsonLexContextOwnsTokens(&lex,=20true);=09/*=20=
must=20not=20leak=20on=20error=20*/=0A+=09lex=20=3D=20=
makeJsonLexContextCstringLen(NULL,=20msg,=20msglen,=20PG_UTF8,=20true);=0A=
+=09setJsonLexContextOwnsTokens(lex,=20true);=09/*=20must=20not=20leak=20=
on=20error=20*/=0A=20=0A=20=09initPQExpBuffer(&ctx.errbuf);=0A=20=09=
sem.semstate=20=3D=20&ctx;=0A@@=20-516,7=20+516,7=20@@=20=
handle_oauth_sasl_error(PGconn=20*conn,=20const=20char=20*msg,=20int=20=
msglen)=0A=20=09sem.array_start=20=3D=20oauth_json_array_start;=0A=20=09=
sem.scalar=20=3D=20oauth_json_scalar;=0A=20=0A-=09err=20=3D=20=
pg_parse_json(&lex,=20&sem);=0A+=09err=20=3D=20pg_parse_json(lex,=20=
&sem);=0A=20=0A=20=09if=20(err=20=3D=3D=20JSON_SEM_ACTION_FAILED)=0A=20=09=
{=0A@@=20-535,7=20+535,7=20@@=20handle_oauth_sasl_error(PGconn=20*conn,=20=
const=20char=20*msg,=20int=20msglen)=0A=20=09=09}=0A=20=09}=0A=20=09else=20=
if=20(err=20!=3D=20JSON_SUCCESS)=0A-=09=09errmsg=20=3D=20=
json_errdetail(err,=20&lex);=0A+=09=09errmsg=20=3D=20json_errdetail(err,=20=
lex);=0A=20=0A=20=09if=20(errmsg)=0A=20=09=09=
libpq_append_conn_error(conn,=0A@@=20-544,7=20+544,7=20@@=20=
handle_oauth_sasl_error(PGconn=20*conn,=20const=20char=20*msg,=20int=20=
msglen)=0A=20=0A=20=09/*=20Don't=20need=20the=20error=20buffer=20or=20=
the=20JSON=20lexer=20anymore.=20*/=0A=20=09termPQExpBuffer(&ctx.errbuf);=0A=
-=09freeJsonLexContext(&lex);=0A+=09freeJsonLexContext(lex);=0A=20=0A=20=09=
if=20(errmsg)=0A=20=09=09goto=20cleanup;=0Adiff=20--git=20=
a/src/test/modules/test_json_parser/test_json_parser_incremental.c=20=
b/src/test/modules/test_json_parser/test_json_parser_incremental.c=0A=
index=20a529ee47e9b..d1e3e4ab4ea=20100644=0A---=20=
a/src/test/modules/test_json_parser/test_json_parser_incremental.c=0A+++=20=
b/src/test/modules/test_json_parser/test_json_parser_incremental.c=0A@@=20=
-84,7=20+84,7=20@@=20main(int=20argc,=20char=20**argv)=0A=20=09char=09=09=
buff[BUFSIZE];=0A=20=09FILE=09=20=20=20*json_file;=0A=20=09=
JsonParseErrorType=20result;=0A-=09JsonLexContext=20lex;=0A+=09=
JsonLexContext=20*lex;=0A=20=09StringInfoData=20json;=0A=20=09int=09=09=09=
n_read;=0A=20=09size_t=09=09chunk_size=20=3D=20DEFAULT_CHUNK_SIZE;=0A@@=20=
-98,6=20+98,10=20@@=20main(int=20argc,=20char=20**argv)=0A=20=0A=20=09=
pg_logging_init(argv[0]);=0A=20=0A+=09lex=20=3D=20calloc(1,=20=
sizeof(JsonLexContext));=0A+=09if=20(!lex)=0A+=09=09pg_fatal("out=20of=20=
memory");=0A+=0A=20=09while=20((c=20=3D=20getopt(argc,=20argv,=20=
"c:os"))=20!=3D=20-1)=0A=20=09{=0A=20=09=09switch=20(c)=0A@@=20-113,7=20=
+117,7=20@@=20main(int=20argc,=20char=20**argv)=0A=20=09=09=09case=20=
's':=09=09=09/*=20do=20semantic=20processing=20*/=0A=20=09=09=09=09=
testsem=20=3D=20&sem;=0A=20=09=09=09=09sem.semstate=20=3D=20=
palloc(sizeof(struct=20DoState));=0A-=09=09=09=09((struct=20DoState=20*)=20=
sem.semstate)->lex=20=3D=20&lex;=0A+=09=09=09=09((struct=20DoState=20*)=20=
sem.semstate)->lex=20=3D=20lex;=0A=20=09=09=09=09((struct=20DoState=20*)=20=
sem.semstate)->buf=20=3D=20makeStringInfo();=0A=20=09=09=09=09=
need_strings=20=3D=20true;=0A=20=09=09=09=09break;=0A@@=20-131,8=20=
+135,8=20@@=20main(int=20argc,=20char=20**argv)=0A=20=09=09exit(1);=0A=20=
=09}=0A=20=0A-=09makeJsonLexContextIncremental(&lex,=20PG_UTF8,=20=
need_strings);=0A-=09setJsonLexContextOwnsTokens(&lex,=20=
lex_owns_tokens);=0A+=09makeJsonLexContextIncremental(lex,=20PG_UTF8,=20=
need_strings);=0A+=09setJsonLexContextOwnsTokens(lex,=20=
lex_owns_tokens);=0A=20=09initStringInfo(&json);=0A=20=0A=20=09if=20=
((json_file=20=3D=20fopen(testfile,=20PG_BINARY_R))=20=3D=3D=20NULL)=0A=
@@=20-165,12=20+169,12=20@@=20main(int=20argc,=20char=20**argv)=0A=20=09=09=
bytes_left=20-=3D=20n_read;=0A=20=09=09if=20(bytes_left=20>=200)=0A=20=09=
=09{=0A-=09=09=09result=20=3D=20pg_parse_json_incremental(&lex,=20=
testsem,=0A+=09=09=09result=20=3D=20pg_parse_json_incremental(lex,=20=
testsem,=0A=20=09=09=09=09=09=09=09=09=09=09=09=20=20=20json.data,=20=
n_read,=0A=20=09=09=09=09=09=09=09=09=09=09=09=20=20=20false);=0A=20=09=09=
=09if=20(result=20!=3D=20JSON_INCOMPLETE)=0A=20=09=09=09{=0A-=09=09=09=09=
fprintf(stderr,=20"%s\n",=20json_errdetail(result,=20&lex));=0A+=09=09=09=
=09fprintf(stderr,=20"%s\n",=20json_errdetail(result,=20lex));=0A=20=09=09=
=09=09ret=20=3D=201;=0A=20=09=09=09=09goto=20cleanup;=0A=20=09=09=09}=0A=
@@=20-178,12=20+182,12=20@@=20main(int=20argc,=20char=20**argv)=0A=20=09=09=
}=0A=20=09=09else=0A=20=09=09{=0A-=09=09=09result=20=3D=20=
pg_parse_json_incremental(&lex,=20testsem,=0A+=09=09=09result=20=3D=20=
pg_parse_json_incremental(lex,=20testsem,=0A=20=09=09=09=09=09=09=09=09=09=
=09=09=20=20=20json.data,=20n_read,=0A=20=09=09=09=09=09=09=09=09=09=09=09=
=20=20=20true);=0A=20=09=09=09if=20(result=20!=3D=20JSON_SUCCESS)=0A=20=09=
=09=09{=0A-=09=09=09=09fprintf(stderr,=20"%s\n",=20=
json_errdetail(result,=20&lex));=0A+=09=09=09=09fprintf(stderr,=20=
"%s\n",=20json_errdetail(result,=20lex));=0A=20=09=09=09=09ret=20=3D=20=
1;=0A=20=09=09=09=09goto=20cleanup;=0A=20=09=09=09}=0A@@=20-195,8=20=
+199,9=20@@=20main(int=20argc,=20char=20**argv)=0A=20=0A=20cleanup:=0A=20=
=09fclose(json_file);=0A-=09freeJsonLexContext(&lex);=0A+=09=
freeJsonLexContext(lex);=0A=20=09free(json.data);=0A+=09free(lex);=0A=20=0A=
=20=09return=20ret;=0A=20}=0A--=20=0A2.39.3=20(Apple=20Git-146)=0A=0A=

--Apple-Mail=_0BAD372B-A8CE-4356-B2C0-344EA112A4D5--