[PATCH] build: clang stack frame size handling improvement
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048 # clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif -- 2.49.0
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}. I worry this is clang version dependent. Should we just drop check for 'optimization' argument altogether? Michal
On Thu, Apr 03, 2025 at 02:08:28PM +0200, Michal Prívozník via Devel wrote:
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}.
I worry this is clang version dependent. Should we just drop check for 'optimization' argument altogether?
We originally picked 2k default in commit 42bc76cdb8486ef502200f3bce9e3faebdd78103 Author: Peter Krempa <pkrempa@redhat.com> Date: Mon Sep 5 14:38:09 2022 +0200 build: Decrease maximum stack frame size to 2048 After recent cleanups we can now restrict the maximum stack frame size to 2k. guess we must be just a bit too aggressive with certain compiler scenarios - with various hardening countermeasures compilers may choose to apply, stack size can be bigger than we might otherwise expect. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Michal Prívozník wrote:
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}.
That's interesting indeed. Currently, "plain" is the only value that triggers the issue for me: Problematic file is the following: ../src/remote/remote_driver.c:790:1: error: stack frame size (2344) exceeds limit (2048) in 'doRemoteOpen' [-Werror,-Wframe-larger-than] 790 | doRemoteOpen(virConnectPtr conn, | ^ 1 error generated. Clang version is 19.1.7. Roman
On 4/3/25 18:28, Roman Bogorodskiy wrote:
Michal Prívozník wrote:
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}.
That's interesting indeed.
Currently, "plain" is the only value that triggers the issue for me:
Problematic file is the following:
../src/remote/remote_driver.c:790:1: error: stack frame size (2344) exceeds limit (2048) in 'doRemoteOpen' [-Werror,-Wframe-larger-than] 790 | doRemoteOpen(virConnectPtr conn, | ^ 1 error generated.
Clang version is 19.1.7.
Same here: clang version 19.1.7 Target: x86_64-pc-linux-gnu except I'm running Linux and I assume you're running *BSD. Given what also Dan said, are you willing to post a v2 where the condition is simplified to just 'clang' and no get_option('optimization')? Actually, give me a minute or to - maybe there's something I can do about doRemoteOpen() so that its stack isn't that huge. Michal
On 4/4/25 08:46, Michal Prívozník wrote:
On 4/3/25 18:28, Roman Bogorodskiy wrote:
Michal Prívozník wrote:
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}.
That's interesting indeed.
Currently, "plain" is the only value that triggers the issue for me:
Problematic file is the following:
../src/remote/remote_driver.c:790:1: error: stack frame size (2344) exceeds limit (2048) in 'doRemoteOpen' [-Werror,-Wframe-larger-than] 790 | doRemoteOpen(virConnectPtr conn, | ^ 1 error generated.
Clang version is 19.1.7.
Same here:
clang version 19.1.7 Target: x86_64-pc-linux-gnu
except I'm running Linux and I assume you're running *BSD. Given what also Dan said, are you willing to post a v2 where the condition is simplified to just 'clang' and no get_option('optimization')?
Actually, give me a minute or to - maybe there's something I can do about doRemoteOpen() so that its stack isn't that huge.
Alight, I think I know what's going on and why we're getting 2300+ bytes long stack even with just a few variables declared. Shortly: it's because of glib. Longer version: glib declares plenty of helper variables (mostly for type checking). For instance: g_clear_pointer() is declared as: #define g_clear_pointer(pp, destroy) \ G_STMT_START \ { \ G_STATIC_ASSERT (sizeof *(pp) == sizeof (gpointer)); \ glib_typeof ((pp)) _pp = (pp); \ glib_typeof (*(pp)) _ptr = *_pp; \ *_pp = NULL; \ if (_ptr) \ (destroy) (_ptr); \ } \ G_STMT_END \ And our VIR_FREE() is defined to be g_clear_pointer(). Then, g_strdup() is defined to be g_strdup_inline(), which: a) is declared as always inline, and b) declares two additional variables: G_ALWAYS_INLINE static inline char * g_strdup_inline (const char *str) { if (__builtin_constant_p (!str) && !str) return NULL; if (__builtin_constant_p (!!str) && !!str && __builtin_constant_p (strlen (str))) { const size_t len = strlen (str) + 1; char *dup_str = (char *) g_malloc (len); return (char *) memcpy (dup_str, str, len); } return g_strdup (str); } And finally, for some reason, attribute cleanup also increases stack size (observed by playing with godbolt.org) - but this is for both gcc and clang and isn't glib related. Honestly, I do not understand why attribute cleanup would need to increase stack size, but whatever. I think now I have all the ingredients needed to break the function down into smaller pieces. So hopefully, in the end, this whole workaround might be dropped. NB: there's still vboxSnapshotRedefine() which has even bigger stack: ../src/vbox/vbox_common.c:4557:1: error: stack frame size (2824) exceeds limit (2048) in 'vboxSnapshotRedefine' [-Werror,-Wframe-larger-than] but I think (wishfully) the same strategy can be applied there. Michal
Michal Prívozník wrote:
On 4/4/25 08:46, Michal Prívozník wrote:
On 4/3/25 18:28, Roman Bogorodskiy wrote:
Michal Prívozník wrote:
On 4/2/25 19:24, Roman Bogorodskiy wrote:
The 'plain' optimization type also triggers the clang stack frame size issues, so increase limit for it as well.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build index 56823ca25b..0a402a19a2 100644 --- a/meson.build +++ b/meson.build @@ -259,7 +259,7 @@ alloc_max = run_command( stack_frame_size = 2048
# clang without optimization enlarges stack frames in certain corner cases -if cc.get_id() == 'clang' and get_option('optimization') == '0' +if cc.get_id() == 'clang' and get_option('optimization') in ['plain', '0'] stack_frame_size = 4096 endif
Funny, with clang I hit this issue for all possible values of --optimization {plain,0,g,1,2,3,s}.
That's interesting indeed.
Currently, "plain" is the only value that triggers the issue for me:
Problematic file is the following:
../src/remote/remote_driver.c:790:1: error: stack frame size (2344) exceeds limit (2048) in 'doRemoteOpen' [-Werror,-Wframe-larger-than] 790 | doRemoteOpen(virConnectPtr conn, | ^ 1 error generated.
Clang version is 19.1.7.
Same here:
clang version 19.1.7 Target: x86_64-pc-linux-gnu
except I'm running Linux and I assume you're running *BSD. Given what also Dan said, are you willing to post a v2 where the condition is simplified to just 'clang' and no get_option('optimization')?
Actually, give me a minute or to - maybe there's something I can do about doRemoteOpen() so that its stack isn't that huge.
Alight, I think I know what's going on and why we're getting 2300+ bytes long stack even with just a few variables declared.
Shortly: it's because of glib.
Longer version: glib declares plenty of helper variables (mostly for type checking). For instance:
g_clear_pointer() is declared as:
#define g_clear_pointer(pp, destroy) \ G_STMT_START \ { \ G_STATIC_ASSERT (sizeof *(pp) == sizeof (gpointer)); \ glib_typeof ((pp)) _pp = (pp); \ glib_typeof (*(pp)) _ptr = *_pp; \ *_pp = NULL; \ if (_ptr) \ (destroy) (_ptr); \ } \ G_STMT_END \
And our VIR_FREE() is defined to be g_clear_pointer().
Then, g_strdup() is defined to be g_strdup_inline(), which: a) is declared as always inline, and b) declares two additional variables:
G_ALWAYS_INLINE static inline char * g_strdup_inline (const char *str) { if (__builtin_constant_p (!str) && !str) return NULL;
if (__builtin_constant_p (!!str) && !!str && __builtin_constant_p (strlen (str))) { const size_t len = strlen (str) + 1; char *dup_str = (char *) g_malloc (len); return (char *) memcpy (dup_str, str, len); }
return g_strdup (str); }
And finally, for some reason, attribute cleanup also increases stack size (observed by playing with godbolt.org) - but this is for both gcc and clang and isn't glib related. Honestly, I do not understand why attribute cleanup would need to increase stack size, but whatever. I think now I have all the ingredients needed to break the function down into smaller pieces. So hopefully, in the end, this whole workaround might be dropped.
NB: there's still vboxSnapshotRedefine() which has even bigger stack:
../src/vbox/vbox_common.c:4557:1: error: stack frame size (2824) exceeds limit (2048) in 'vboxSnapshotRedefine' [-Werror,-Wframe-larger-than]
but I think (wishfully) the same strategy can be applied there.
IIRC, the first time I've encountered this issue was actually a test file. So I guess it would still make sense to keep the workaround for clang for now, dropping the optimization condition? Roman
On 4/4/25 11:02, Roman Bogorodskiy wrote:
Michal Prívozník wrote:
<snip/>
IIRC, the first time I've encountered this issue was actually a test file. So I guess it would still make sense to keep the workaround for clang for now, dropping the optimization condition?
Maybe, but I haven't found anything there. I've tested these successfully on my FreeBSD VM: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/XT2HH... Michal
participants (3)
-
Daniel P. Berrangé -
Michal Prívozník -
Roman Bogorodskiy