ASoC: SOF: ipc4-topology: Fix use of unitialized value

[dbaluta]

Initialize ref_params to zero in order to fix static analyzer error:

ipc4-topology.c: In function ‘sof_ipc4_prepare_copier_module’: ipc4-topology.c:1810:34: error: use of uninitialized value ‘((int *)<unknown>)[2]’ [CWE-457] [-Werror=analyzer-use-of-uninitialized-value] struct snd_pcm_hw_params ref_params;

Note that this is a false positive, as manually inspecting the code there is no path where ref_params is used unitialized.

Fixes: f9209644ae76 ("ASoC: SOF: ipc4-topology: Correct DAI copier config and NHLT blob request")

[ujfalusi]

This is so false report, I have refused to fix it out of principle ;)
ref_params is going to be initialized and when it is not we will just return with -EINVAL.
See the first switch (swidget->id) cases...

[dbaluta]

Yes, this is a false positive. I wonder if we can mark it somehow so that the CI report

 build test / GCC static -fanalyzer (pull_request) Failing after 3m 

would be useful.

[ujfalusi]

if nothing else we should take this patch..

[dbaluta]

@ujfalusi i noticed that there are at least 3-4 cases of similar false positives mostly in @thesofproject/amd code.

I have no strong preference or merging this patch.

Cc: @ranj063 @lgirdwood if you consider this useful you can merge it.

[ujfalusi]

@dbaluta, fwiw, with

$ gcc --version                                 
gcc (GCC) 14.2.1 20240910
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

-fanalyzer no longer reports false positive for this.

[dbaluta]

@ujfalusi that's good news.

We are already using latest ubuntu, so at some point in the future the compiler version will be upgraded

Would anyone care to use a newer compiler version in .github/workflows/buildtest.yml?

[bardliao]

I wish we can switch to newer compiler version. :)

[ujfalusi]

I'll try to figure this out

[dbaluta]

Will close this and mark it as known issue.

[ujfalusi]

It also triggers with gcc-14 and by adding -fanalyzer-verbosity=4 it reveals what I still don't understand:

sound/soc/sof/ipc4-topology.c: In function ‘sof_ipc4_prepare_copier_module’:
sound/soc/sof/ipc4-topology.c:1810:34: error: use of uninitialized value ‘((int *)<unknown>)[2]’ [CWE-457] [-Werror=analyzer-use-of-uninitialized-value]
 1810 |         struct snd_pcm_hw_params ref_params;
      |                                  ^~~~~~~~~~
  ‘sof_ipc4_prepare_copier_module’: events 1-3
    |
    | 1800 | sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
    |      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      | |
    |      | (1) entry to ‘sof_ipc4_prepare_copier_module’
    |......
    | 1810 |         struct snd_pcm_hw_params ref_params;
    |      |                                  ~~~~~~~~~~
    |      |                                  |
    |      |                                  (2) region created on stack here
    |      |                                  (3) region creation: ref_params+(struct snd_pcm_hw_params *)268B
    |
  ‘sof_ipc4_prepare_copier_module’: event 4
    |
    |cc1:
    | (4): 
    |
  ‘sof_ipc4_prepare_copier_module’: events 5-7
    |
    | 1806 |         struct snd_soc_component *scomp = swidget->scomp;
    |      |                                   ^~~~~
    |      |                                   |
    |      |                                   (5) ...to here
    |      |                                   (6) stmt: scomp_237 = swidget_236(D)->scomp;
    | 1807 |         struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
    |      |                                    ~
    |      |                                    |
    |      |                                    (7) inlined call to ‘snd_soc_component_get_drvdata’ from ‘sof_ipc4_prepare_copier_module’
    |
    +--> ‘snd_soc_component_get_drvdata’: events 8-9
           |
           |./include/sound/soc-component.h:434:33:
           |  434 |         return dev_get_drvdata(c->dev);
           |      |                ~               ~^~~~~
           |      |                |                |
           |      |                |                (8) stmt: _516 = scomp_237->dev;
           |      |                (9) inlined call to ‘dev_get_drvdata’ from ‘snd_soc_component_get_drvdata’
           |
           +--> ‘dev_get_drvdata’: event 10
                  |
                  |./include/linux/device.h:940:19:
                  |  940 |         return dev->driver_data;
                  |      |                ~~~^~~~~~~~~~~~~
                  |      |                   |
                  |      |                   (10) stmt: _517 = MEM[(const struct device *)_516].driver_data;
                  |
    <-------------+
    |
  ‘sof_ipc4_prepare_copier_module’: events 11-12
    |
    |sound/soc/sof/ipc4-topology.c:1810:34:
    | 1810 |         struct snd_pcm_hw_params ref_params;
    |      |                                  ^~~~~~~~~~
    |      |                                  |
    |      |                                  (11) stmt: ref_params = .DEFERRED_INIT (608, 1, &"ref_params"[0]);
    |      |                                  (12) call summary
    |
  ‘sof_ipc4_prepare_copier_module’: event 13
    |
    |cc1:
    | (13): 
    |
  ‘sof_ipc4_prepare_copier_module’: events 14-19
    |
    | 1810 |         struct snd_pcm_hw_params ref_params;
    |      |                                  ^~~~~~~~~~
    |      |                                  |
    |      |                                  (14) ...to here
    |      |                                  (15) stmt: if (&MEM <struct snd_pcm_hw_params> [(void *)&ref_params + 404B] != _196)
    |      |                                  (16) following ‘true’ branch...
    |      |                                  (17) ...to here
    |      |                                  (18) stmt: _168 = MEM <int> [(struct snd_pcm_hw_params *)_196 + 8B];
    |      |                                  (19) use of uninitialized value ‘((int *)<unknown>)[2]’ here
    |

and btw, if this is 'fixed' then the gcc static analyzer will fail in sound/soc/intel/boards/sof_sdw.c with another uninitialized variable, which again looks like a false positive.