Re: [PATCH 8/8] build: Decrease maximum stack frame size to 2048

On Mon, Sep 04, 2023 at 16:09:00 +0200, Michal Prívozník wrote: > On 8/30/23 13:59, Peter Krempa wrote: > > After recent cleanups we can now restrict the maximum stack frame size > > to 2k. > > > > Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; > > --- > > meson.build | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meson.build b/meson.build > > index 965ada483b..e45f3e2c39 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -248,7 +248,7 @@ alloc_max = run_command( > > ) > > > > # sanitizer instrumentation may enlarge stack frames > > -stack_frame_size = get_option('b_sanitize') == 'none' ? 4096 : 32768 > > +stack_frame_size = get_option('b_sanitize') == 'none' ? 2048 : 32768 > > > > # array_bounds=2 check triggers false positive on some GCC > > # versions when using sanitizers. Seen on Fedora 34 with > > > I know this is already pushed but to be honest, I don't really > understand why this is needed. I mean, we certainly do not want large > frames, but IIUC only frames larger than a page are problem (i.e. 4KiB). > Can you please point me to some docs where I can learn more? The main idea of this is to ensure that we don't have massive stack frames which could cause a stack overflow. But to be honest, it doesn't matter too much whether the actual limit size is 4k or 2k. It still allows us to have very deep call stack, and a factor of 2 in the size will not make a real difference. At one point I was looking at which functions have massive stack frames and tried avoiding such state. Now this last set of patches was what I had in my local branch for a long time and decided to finally get rid of them. As you can see, there were multiple cases of 2k buffers being stack allocated, so the end result IMO made sense. The decreasing of the actual limit to 2k isn't as important as I've noted but similarly to when we add a syntax check after a full-repo cleanup it is a way to prevent regressions after a cleanup.