Re: Should we add a compiler warning for large stack frames? - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Should we add a compiler warning for large stack frames?
Date
Msg-id 20240411220711.7agp2qzqub7olkow@awork3.anarazel.de
Whole thread Raw
In response to Re: Should we add a compiler warning for large stack frames?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Should we add a compiler warning for large stack frames?
Re: Should we add a compiler warning for large stack frames?
List pgsql-hackers
Hi,

On 2024-04-11 16:35:58 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > d8f5acbdb9b made me wonder if we should add a compiler option to warn when
> > stack frames are large.  gcc compatible compilers have -Wstack-usage=limit, so
> > that's not hard.
>
> > Huge stack frames are somewhat dangerous, as they can defeat our stack-depth
> > checking logic. There are also some cases where large stack frames defeat
> > stack-protector logic by compilers/libc/os.
>
> Indeed.  I recall reading, not long ago, some Linux kernel docs to the
> effect that automatic stack growth is triggered by a reference into
> the page just below what is currently mapped as your stack, and
> therefore allocating a stack frame greater than one page has the
> potential to cause SIGSEGV rather than the desired stack extension.
> (If you feel like digging in the archives, I think this was brought
> up in the last round of lets-add-some-more-check_stack_depth-calls.)

I think it's more than a single page, but I'm not entirely sure either. I
think some compilers inject artificial stack accesses when extending the stack
by a lot, but I don't remember the details.

There certainly was the issue that strict memory overcommit does not reliably
work with larger stack extensions.

Could be worth writing a test program for...


> > I don't really have an opinion about the concrete warning limit to use.
>
> Given the above, I'm tempted to say we should make it 8K.

Hm, why 8k? That's 2x the typical page size, isn't it?


> But I know we have a bunch of places that allocate page images as stack
> space, so maybe that'd be too painful.

8k does generate a fair number of warnings, 111 here.  I think it might also
be hard to ensure that inlining doesn't end up creating bigger stack frames.

frame size      warnings
4096            155
8192            111
16384           36
32768           14
65536           8

Suggests that starting somewhere around 16-32k might be reasonable?

One issue is of course that configuring a larger blocksize or wal_blocksize
will trigger more warnings. I guess we'd need to set the limit based on
Max(blocksize, wal_blocksize) * 2 or such.

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Table AM Interface Enhancements
Next
From: David Steele
Date:
Subject: Re: Add notes to pg_combinebackup docs