Thread

New compiler warning in jsonb_plpython.c

Tom Lane <tgl@sss.pgh.pa.us> — 2020-01-28T00:12:46Z

I see that buildfarm member caiman is generating a warning [1]:

jsonb_plpython.c: In function \xe2\x80\x98PLyObject_ToJsonbValue\xe2\x80\x99:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
jsonb_plpython.c:413:13: note: declared here
  413 |  JsonbValue buf;
      |             ^~~

It wasn't doing that a week or two ago when I last trawled the buildfarm
for warnings ... but this is unsurprising considering that the compiler
it's using is hot off the presses:

configure: using compiler=gcc (GCC) 10.0.1 20200121 (Red Hat 10.0.1-0.4)

The warning is from code like this:

{
    JsonbValue  buf;
    JsonbValue *out;
    ...
    if (*jsonb_state)
        out = &buf;
    else
        out = palloc(sizeof(JsonbValue));
    ...
    return (*jsonb_state ?
            pushJsonbValue(jsonb_state, is_elem ? WJB_ELEM : WJB_VALUE, out) :
            out);
}

so I can't say I blame gcc for being unhappy.  This code is safe as long
as *jsonb_state doesn't change in between, and as long as pushJsonbValue
doesn't expect its last argument to point at non-transient storage.  But
gcc doesn't want to assume that, and I don't really like the assumption
either.

I am thinking of trying to silence the warning by changing the return
to be like

    return (out == &buf ?
            pushJsonbValue(jsonb_state, is_elem ? WJB_ELEM : WJB_VALUE, out) :
            out);

If that doesn't work, or if anyone thinks it's too ugly, I think we
should just drop the optimization of avoiding a palloc, and make
this function do a palloc always.  It seems unlikely that anyone
would notice a performance difference, and the code would surely
be less rickety.

Thoughts?

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=caiman&dt=2020-01-25%2015%3A00%3A52