Silencing upcoming warning about stack_base_ptr

Started by Tom Laneabout 4 years ago3 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

GCC 12, coming soon to a distro near you, complains like this:

postgres.c: In function 'set_stack_base':
postgres.c:3430:24: warning: storing the address of local variable 'stack_base' in 'stack_base_ptr' [-Wdangling-pointer=]
3430 | stack_base_ptr = &stack_base;
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~
postgres.c:3419:25: note: 'stack_base' declared here
3419 | char stack_base;
| ^~~~~~~~~~
postgres.c:136:13: note: 'stack_base_ptr' declared here
136 | char *stack_base_ptr = NULL;
| ^~~~~~~~~~~~~~

(that's visible now on buildfarm member caiman). We probably
should take some thought for silencing this before it starts
to be in people's faces during routine development.

Fixing this is a bit harder than one could wish because we export
set_stack_base() for use by PL/Java, so it would be better to not
change that API. I ended up with the attached patch, which works
to silence the warning so long as the new subroutine
set_stack_base_from() is marked pg_noinline. I'm a little worried
that in a year or two GCC will be smart enough to complain anyway.
If that happens, we could probably silence the warning again by
moving set_stack_base() to a different source file --- but at some
point we might have to give up and change its API, I suppose.

I also took this opportunity to re-static-ify the stack_base_ptr
variable itself, as PL/Java seems to have adopted set_stack_base
since 1.5.0. That part perhaps should not be back-patched.

regards, tom lane

Attachments:

fix-stack-base-warning.patchtext/x-diff; charset=us-ascii; name=fix-stack-base-warning.patchDownload+38-18
#2Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#1)
Re: Silencing upcoming warning about stack_base_ptr

Hi,

On 2022-02-17 18:44:27 -0500, Tom Lane wrote:

(that's visible now on buildfarm member caiman). We probably
should take some thought for silencing this before it starts
to be in people's faces during routine development.

Agreed.

One annoying thing I recently encountered, related to this, is that our stack
check fails to work with some -fsanitize* options because they end up using
multiple stacks (e.g. -fsanitize-address-use-after-scope). Not sure we can
really do anything about that...

Fixing this is a bit harder than one could wish because we export
set_stack_base() for use by PL/Java, so it would be better to not
change that API. I ended up with the attached patch, which works
to silence the warning so long as the new subroutine
set_stack_base_from() is marked pg_noinline. I'm a little worried
that in a year or two GCC will be smart enough to complain anyway.
If that happens, we could probably silence the warning again by
moving set_stack_base() to a different source file --- but at some
point we might have to give up and change its API, I suppose.

We could try using __builtin_frame_address(0) when available, presumably gcc
won't warn about that...

Greetings,

Andres Freund

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: Silencing upcoming warning about stack_base_ptr

Andres Freund <andres@anarazel.de> writes:

On 2022-02-17 18:44:27 -0500, Tom Lane wrote:

(that's visible now on buildfarm member caiman). We probably
should take some thought for silencing this before it starts
to be in people's faces during routine development.

We could try using __builtin_frame_address(0) when available, presumably gcc
won't warn about that...

Oh, I didn't know about that. Seems like a better option since
then we don't need any API changes at all. Maybe at some point
some non-gcc-alike will start delivering a comparable warning,
but then we could fall back to what I did here.

regards, tom lane