[MASSMAIL]Should we add a compiler warning for large stack frames?

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
66976bytes [-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
131232bytes [-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
524400bytes [-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