Re: Should we add a compiler warning for large stack frames?
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Andres Freund <andres@anarazel.de>, pgsql-hackers@postgresql.org,
Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>
Date: 2024-04-11T19:16:57Z
Lists: pgsql-hackers
On 2024-04-11 Th 15:01, Andres Freund wrote: > 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. > > > [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, > | ^~~~~~~~~~~~~~~~~~~~ > This one's down to me. I asked Robert some time back why we were using a very conservative buffer size, and he agreed we could probably make it larger, but the number chosen is mine, not his. It was a completely arbitrary choice. I'm happy to reduce it, but it's not clear to me why we care that much for a client binary. There's no stack depth checking going on here. cheers andrew -- Andrew Dunstan EDB:https://www.enterprisedb.com
Commits
-
Don't allocate large buffer on the stack in pg_verifybackup
- 929c05774b51 17.0 landed