public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: [email protected]
To: Robert Haas <[email protected]>
To: Thomas Munro <[email protected]>
Subject: Should we add a compiler warning for large stack frames?
Date: Thu, 11 Apr 2024 12:01:47 -0700
Message-ID: <[email protected]> (raw)
Hi,
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.
It's not always obvious how large the stack will be. Even if you look at all
the sizes of the variables defined in a function, inlining can increase that
substantially.
Here are all the cases a limit of 64k finds.
Unoptimized build:
[138/1940 42 7%] Compiling C object src/common/libpgcommon_shlib.a.p/blkreftable.c.o
../../../../../home/andres/src/postgresql/src/common/blkreftable.c: In function 'WriteBlockRefTable':
../../../../../home/andres/src/postgresql/src/common/blkreftable.c:474:1: warning: stack usage is 65696 bytes [-Wstack-usage=]
474 | WriteBlockRefTable(BlockRefTable *brtab,
| ^~~~~~~~~~~~~~~~~~
[173/1940 42 8%] Compiling C object src/common/libpgcommon.a.p/blkreftable.c.o
../../../../../home/andres/src/postgresql/src/common/blkreftable.c: In function 'WriteBlockRefTable':
../../../../../home/andres/src/postgresql/src/common/blkreftable.c:474:1: warning: stack usage is 65696 bytes [-Wstack-usage=]
474 | WriteBlockRefTable(BlockRefTable *brtab,
| ^~~~~~~~~~~~~~~~~~
[281/1940 42 14%] Compiling C object src/common/libpgcommon_srv.a.p/blkreftable.c.o
../../../../../home/andres/src/postgresql/src/common/blkreftable.c: In function 'WriteBlockRefTable':
../../../../../home/andres/src/postgresql/src/common/blkreftable.c:474:1: warning: stack usage is 65696 bytes [-Wstack-usage=]
474 | WriteBlockRefTable(BlockRefTable *brtab,
| ^~~~~~~~~~~~~~~~~~
[1311/1940 42 67%] Compiling C object src/bin/pg_basebackup/pg_basebackup.p/pg_basebackup.c.o
../../../../../home/andres/src/postgresql/src/bin/pg_basebackup/pg_basebackup.c: In function 'BaseBackup':
../../../../../home/andres/src/postgresql/src/bin/pg_basebackup/pg_basebackup.c:1753:1: warning: stack usage might be 66976 bytes [-Wstack-usage=]
1753 | BaseBackup(char *compression_algorithm, char *compression_detail,
| ^~~~~~~~~~
[1345/1940 42 69%] Compiling C object src/bin/pg_verifybackup/pg_verifybackup.p/pg_verifybackup.c.o
../../../../../home/andres/src/postgresql/src/bin/pg_verifybackup/pg_verifybackup.c: In function 'verify_file_checksum':
../../../../../home/andres/src/postgresql/src/bin/pg_verifybackup/pg_verifybackup.c:842:1: warning: stack usage is 131232 bytes [-Wstack-usage=]
842 | verify_file_checksum(verifier_context *context, manifest_file *m,
| ^~~~~~~~~~~~~~~~~~~~
[1349/1940 42 69%] Compiling C object src/bin/pg_waldump/pg_waldump.p/pg_waldump.c.o
../../../../../home/andres/src/postgresql/src/bin/pg_waldump/pg_waldump.c: In function 'main':
../../../../../home/andres/src/postgresql/src/bin/pg_waldump/pg_waldump.c:792:1: warning: stack usage might be 105104 bytes [-Wstack-usage=]
792 | main(int argc, char **argv)
| ^~~~
[1563/1940 42 80%] Compiling C object contrib/pg_walinspect/pg_walinspect.so.p/pg_walinspect.c.o
../../../../../home/andres/src/postgresql/contrib/pg_walinspect/pg_walinspect.c: In function 'GetWalStats':
../../../../../home/andres/src/postgresql/contrib/pg_walinspect/pg_walinspect.c:762:1: warning: stack usage is 104624 bytes [-Wstack-usage=]
762 | GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn,
| ^~~~~~~~~~~
[1581/1940 42 81%] Compiling C object src/test/modules/test_dsa/test_dsa.so.p/test_dsa.c.o
../../../../../home/andres/src/postgresql/src/test/modules/test_dsa/test_dsa.c: In function 'test_dsa_resowners':
../../../../../home/andres/src/postgresql/src/test/modules/test_dsa/test_dsa.c:64:1: warning: stack usage is 80080 bytes [-Wstack-usage=]
64 | test_dsa_resowners(PG_FUNCTION_ARGS)
| ^~~~~~~~~~~~~~~~~~
There is one warning that is just visible in an optimized build,
otherwise the precise amount of stack usage just differs some:
[1165/2017 42 57%] Compiling C object src/backend/postgres_lib.a.p/tsearch_spell.c.o
../../../../../home/andres/src/postgresql/src/backend/tsearch/spell.c: In function 'NIImportAffixes':
../../../../../home/andres/src/postgresql/src/backend/tsearch/spell.c:1425:1: warning: stack usage might be 74080 bytes [-Wstack-usage=]
1425 | NIImportAffixes(IspellDict *Conf, const char *filename)
| ^~~~~~~~~~~~~~~
Warnings in src/bin aren't as interesting as warnings in backend code, as the
latter is much more likely to be "exposed" to deep stacks and could be
vulnerable due to stack overflows.
I did verify this would have warned about d8f5acbdb9b^:
[1/2 1 50%] Compiling C object src/backend/postgres_lib.a.p/backup_basebackup_incremental.c.o
../../../../../home/andres/src/postgresql/src/backend/backup/basebackup_incremental.c: In function 'GetFileBackupMethod':
../../../../../home/andres/src/postgresql/src/backend/backup/basebackup_incremental.c:742:1: warning: stack usage is 524400 bytes [-Wstack-usage=]
742 | GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path,
| ^~~~~~~~~~~~~~~~~~~
I don't really have an opinion about the concrete warning limit to use.
Greetings,
Andres Freund
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Should we add a compiler warning for large stack frames?
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox