On 7/20/2022 8:52 PM, Nathan Chancellor wrote:
When building ARCH=arm64 allmodconfig with clang, there is a warning
about high stack usage in avs_path_create(), which breaks the build due
to CONFIG_WERROR=y:
sound/soc/intel/avs/path.c:815:18: error: stack frame size (2176) exceeds limit (2048) in 'avs_path_create' [-Werror,-Wframe-larger-than]
struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
^
1 error generated.
This warning is also visible with allmodconfig on other architectures.
The minimum set of configs that triggers this on top of ARCH=arm64
allnoconfig:
CONFIG_COMPILE_TEST=y
CONFIG_FORTIFY_SOURCE=y
CONFIG_KASAN=y
CONFIG_PCI=y
CONFIG_SOUND=y
CONFIG_SND=y
CONFIG_SND_SOC=y
CONFIG_SND_SOC_INTEL_AVS=y
When CONFIG_FORTIFY_SOURCE is enabled, memcmp() (called from
guid_equal()) becomes a wrapper to do compile time checking, which
interacts poorly with inlining plus CONFIG_KASAN=y.
With ARCH=arm64 allmodconfig + CONFIG_KASAN=n + CONFIG_FRAME_WARN=128,
the stack usage is much better:
sound/soc/intel/avs/path.c:815:18: warning: stack frame size (624) exceeds limit (128) in 'avs_path_create' [-Wframe-larger-than]
struct avs_path *avs_path_create(struct avs_dev *adev, u32 dma_id,
^
sound/soc/intel/avs/path.c:873:5: warning: stack frame size (144) exceeds limit (128) in 'avs_path_bind' [-Wframe-larger-than]
int avs_path_bind(struct avs_path *path)
^
2 warnings generated.
To avoid this warning, mark avs_path_module_type_create() as
noinline_for_stack, which redistributes the stack usage across multiple
functions, regardless of CONFIG_KASAN.
With ARCH=arm64 allmodconfig + CONFIG_FRAME_WARN=128, the warnings show:
avs_path_create(): 192
avs_path_bind(): 272
avs_path_module_type_create(): 416
avs_mux_create(): 160
avs_updown_mix_create(): 160
avs_aec_create(): 176
avs_asrc_create(): 144
With ARCH=arm64 allmodconfig + CONFIG_FRAME_WARN=128 + CONFIG_KASAN=n,
the warnings show:
avs_path_create(): 192
avs_path_bind(): 144
avs_path_module_type_create(): 416
avs_mux_create(): 176
avs_updown_mix_create(): 176
avs_src_create(): 144
avs_aec_create(): 192
avs_asrc_create(): 144
avs_wov_create(): 144
Link: https://github.com/ClangBuiltLinux/linux/issues/1642
Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
---
sound/soc/intel/avs/path.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/avs/path.c b/sound/soc/intel/avs/path.c
index 3d46dd5e5bc4..ec2aa0001f91 100644
--- a/sound/soc/intel/avs/path.c
+++ b/sound/soc/intel/avs/path.c
@@ -449,7 +449,8 @@ static int avs_modext_create(struct avs_dev *adev, struct avs_path_module *mod)
return ret;
}
-static int avs_path_module_type_create(struct avs_dev *adev, struct avs_path_module *mod)
+static noinline_for_stack int avs_path_module_type_create(struct avs_dev *adev,
+ struct avs_path_module *mod)
{
const guid_t *type = &mod->template->cfg_ext->type;
base-commit: ff6992735ade75aae3e35d16b17da1008d753d28
Not a fan of this.
My first question would be what clang does differently in this
configuration (ARM) than in all other configurations (x86, etc.) and gcc.
Overall as evidenced by:
> CONFIG_COMPILE_TEST=y
this is test only and this commit doesn't fix anything for x86 this
driver targets.
Based on description in message and in github link:
Looking at avs_path_module_type_create() it uses guid_equal() which is
marked as inline, but is just a wrapper around memcmp(), which in case
of fortify is still marked as inline... memcmp itself has 2 size_t
variables for performing fortify check... no matter how I calculate, it
shouldn't go above stack size, unless clang decides to also inline all
calls to static avs_xxx_create functions. They are not marked as inline
or noinline, so in theory compiler is free to do whatever it wants, but
apparently it goes wrong way? Of course the above may be wrong, because
I just analyzed code, not real output of clang.
Anyway it is probably ok, to do this, as while it needs to be fast
module creation is not really time critical, and some time will be spend
to communicate with DSP instead of calculating things, but still wonder
if there isn't something that can be done on compiler side...
