On Thu, Dec 9, 2021 at 11:52 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > From: Arnd Bergmann <arnd@xxxxxxxx> > > clang warns about excessive stack usage in this driver when > UBSAN is enabled: > > drivers/staging/greybus/audio_topology.c:977:12: error: stack frame size of 1836 bytes in function 'gbaudio_tplg_create_widget' [-Werror,-Wframe-larger-than=] > > Rework this code to no longer use compound literals for > initializing the structure in each case, but instead keep > the common bits in a preallocated constant array and copy > them as needed. > > Link: https://github.com/ClangBuiltLinux/linux/issues/1535 > Link: https://lore.kernel.org/r/20210103223541.2790855-1-arnd@xxxxxxxxxx/ > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> > [nathan: Address review comments from v1] > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx> Thanks for helping to get this across the finish line. Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > --- > > v1 -> v2: https://lore.kernel.org/r/20210103223541.2790855-1-arnd@xxxxxxxxxx/ > > * Use NULL for name field in SND_DOC_DAPM_* in gbaudio_widgets (Alex). > > * Do not eliminate *dw assignment within the switch cases, as invalid > enum values in-between valid enum values (such as snd_soc_dapm_demux) > would not be handled properly by the "if value is greater than the > array size" check (Alex). This addresses a few other comments by Alex > and Dan because w->type is not checked against the array's size. > > Arnd, if you disagree with this approach, please let me know so that we > can get this fixed in a way that everyone is happy with. > > drivers/staging/greybus/audio_topology.c | 92 ++++++++++++------------ > 1 file changed, 45 insertions(+), 47 deletions(-) > > diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c > index 1e613d42d823..7f7d558b76d0 100644 > --- a/drivers/staging/greybus/audio_topology.c > +++ b/drivers/staging/greybus/audio_topology.c > @@ -974,6 +974,44 @@ static int gbaudio_widget_event(struct snd_soc_dapm_widget *w, > return ret; > } > > +static const struct snd_soc_dapm_widget gbaudio_widgets[] = { > + [snd_soc_dapm_spk] = SND_SOC_DAPM_SPK(NULL, gbcodec_event_spk), > + [snd_soc_dapm_hp] = SND_SOC_DAPM_HP(NULL, gbcodec_event_hp), > + [snd_soc_dapm_mic] = SND_SOC_DAPM_MIC(NULL, gbcodec_event_int_mic), > + [snd_soc_dapm_output] = SND_SOC_DAPM_OUTPUT(NULL), > + [snd_soc_dapm_input] = SND_SOC_DAPM_INPUT(NULL), > + [snd_soc_dapm_switch] = SND_SOC_DAPM_SWITCH_E(NULL, SND_SOC_NOPM, > + 0, 0, NULL, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > + [snd_soc_dapm_pga] = SND_SOC_DAPM_PGA_E(NULL, SND_SOC_NOPM, > + 0, 0, NULL, 0, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > + [snd_soc_dapm_mixer] = SND_SOC_DAPM_MIXER_E(NULL, SND_SOC_NOPM, > + 0, 0, NULL, 0, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > + [snd_soc_dapm_mux] = SND_SOC_DAPM_MUX_E(NULL, SND_SOC_NOPM, > + 0, 0, NULL, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > + [snd_soc_dapm_aif_in] = SND_SOC_DAPM_AIF_IN_E(NULL, NULL, 0, > + SND_SOC_NOPM, 0, 0, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > + [snd_soc_dapm_aif_out] = SND_SOC_DAPM_AIF_OUT_E(NULL, NULL, 0, > + SND_SOC_NOPM, 0, 0, > + gbaudio_widget_event, > + SND_SOC_DAPM_PRE_PMU | > + SND_SOC_DAPM_POST_PMD), > +}; > + > static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, > struct snd_soc_dapm_widget *dw, > struct gb_audio_widget *w, int *w_size) > @@ -1052,77 +1090,37 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module, > > switch (w->type) { > case snd_soc_dapm_spk: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_SPK(w->name, gbcodec_event_spk); > + *dw = gbaudio_widgets[w->type]; > module->op_devices |= GBAUDIO_DEVICE_OUT_SPEAKER; > break; > case snd_soc_dapm_hp: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_HP(w->name, gbcodec_event_hp); > + *dw = gbaudio_widgets[w->type]; > module->op_devices |= (GBAUDIO_DEVICE_OUT_WIRED_HEADSET > | GBAUDIO_DEVICE_OUT_WIRED_HEADPHONE); > module->ip_devices |= GBAUDIO_DEVICE_IN_WIRED_HEADSET; > break; > case snd_soc_dapm_mic: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_MIC(w->name, gbcodec_event_int_mic); > + *dw = gbaudio_widgets[w->type]; > module->ip_devices |= GBAUDIO_DEVICE_IN_BUILTIN_MIC; > break; > case snd_soc_dapm_output: > - *dw = (struct snd_soc_dapm_widget)SND_SOC_DAPM_OUTPUT(w->name); > - break; > case snd_soc_dapm_input: > - *dw = (struct snd_soc_dapm_widget)SND_SOC_DAPM_INPUT(w->name); > - break; > case snd_soc_dapm_switch: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_SWITCH_E(w->name, SND_SOC_NOPM, 0, 0, > - widget_kctls, > - gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > - break; > case snd_soc_dapm_pga: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_PGA_E(w->name, SND_SOC_NOPM, 0, 0, NULL, 0, > - gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > - break; > case snd_soc_dapm_mixer: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_MIXER_E(w->name, SND_SOC_NOPM, 0, 0, NULL, > - 0, gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > - break; > case snd_soc_dapm_mux: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_MUX_E(w->name, SND_SOC_NOPM, 0, 0, > - widget_kctls, gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > + *dw = gbaudio_widgets[w->type]; > break; > case snd_soc_dapm_aif_in: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_AIF_IN_E(w->name, w->sname, 0, > - SND_SOC_NOPM, > - 0, 0, gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > - break; > case snd_soc_dapm_aif_out: > - *dw = (struct snd_soc_dapm_widget) > - SND_SOC_DAPM_AIF_OUT_E(w->name, w->sname, 0, > - SND_SOC_NOPM, > - 0, 0, gbaudio_widget_event, > - SND_SOC_DAPM_PRE_PMU | > - SND_SOC_DAPM_POST_PMD); > + *dw = gbaudio_widgets[w->type]; > + dw->sname = w->sname; > break; > default: > ret = -EINVAL; > goto error; > } > + dw->name = w->name; > > dev_dbg(module->dev, "%s: widget of type %d created\n", dw->name, > dw->id); > > base-commit: 42eb8fdac2fc5d62392dcfcf0253753e821a97b0 > -- > 2.34.1 > > -- Thanks, ~Nick Desaulniers
