2022-10-06 03:36:11

by Guru Das Srinagesh

[permalink] [raw]
Subject: [PATCH 2/2] scripts/clang-tools: Remove core.uninitialized.Assign check

Remove this check as it leads to false positives in some cases (not all):

warning: Assigned value is garbage or undefined
[clang-analyzer-core.uninitialized.Assign]
list_for_each_entry_safe(page, tmp_page, &pages, lru)
^

Signed-off-by: Guru Das Srinagesh <[email protected]>
---
scripts/clang-tools/run-clang-tools.py | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
index a72c4c7..714cb82 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -54,6 +54,7 @@ def run_analysis(entry):
# List of checks to be excluded
exclude = []
exclude.append("-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
+ exclude.append("-clang-analyzer-core.uninitialized.Assign")

checks += ''.join(["," + e for e in exclude])
p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
--
2.7.4


2022-10-10 17:57:42

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] scripts/clang-tools: Remove core.uninitialized.Assign check

On Wed, Oct 5, 2022 at 8:24 PM Guru Das Srinagesh
<[email protected]> wrote:
>
> Remove this check as it leads to false positives in some cases (not all):
>
> warning: Assigned value is garbage or undefined
> [clang-analyzer-core.uninitialized.Assign]
> list_for_each_entry_safe(page, tmp_page, &pages, lru)
> ^

I don't think we want to disable this. Tom and others have fixed bugs
from this report. See also:
commit d1bd5fa07667 ("lib: remove back_str initialization")
commit 33b5bc9e7033 ("octeontx2-af: initialize action variable")
commit 8d783197f06d ("mctp: Fix warnings reported by clang-analyzer")
commit eed1a5c74216 ("drm/amdgpu: check return status before using
stable_pstate")
commit 3da4b7403db8 ("ALSA: usb-audio: initialize variables that could
ignore errors")
commit 38ac2f038666 ("iio: chemical: sunrise_co2: set val parameter
only on success")
commit afe6949862f7 ("afs: check function return")
commit d108370c644b ("apparmor: fix error check")
commit d52e419ac8b5 ("rxrpc: Fix handling of an unsupported token type
in rxrpc_read()")
commit 6a6516c024bb ("USB: storage: avoid use of uninitialized values
in error path")
commit f71e41e23e12 ("iio:imu:st_lsm6dsx: check
st_lsm6dsx_shub_read_output return")
commit 3a61cdf43e67 ("hwrng: intel - cleanup initialization")
commit 094dd0d73062 ("rndis_wlan: tighten check of rndis_query_oid return")
commit e3914ed6cf44 ("ieee802154/adf7242: check status of adf7242_read_reg")
commit c2a3d4b4cac1 ("net/mlx4_en: Cleanups suggested by clang static checker")


>
> Signed-off-by: Guru Das Srinagesh <[email protected]>
> ---
> scripts/clang-tools/run-clang-tools.py | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index a72c4c7..714cb82 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -54,6 +54,7 @@ def run_analysis(entry):
> # List of checks to be excluded
> exclude = []
> exclude.append("-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
> + exclude.append("-clang-analyzer-core.uninitialized.Assign")
>
> checks += ''.join(["," + e for e in exclude])
> p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
> --
> 2.7.4
>


--
Thanks,
~Nick Desaulniers

2022-10-10 17:58:47

by Guru Das Srinagesh

[permalink] [raw]
Subject: Re: [PATCH 2/2] scripts/clang-tools: Remove core.uninitialized.Assign check

On Oct 10 2022 10:22, Nick Desaulniers wrote:
> On Wed, Oct 5, 2022 at 8:24 PM Guru Das Srinagesh
> <[email protected]> wrote:
> >
> > Remove this check as it leads to false positives in some cases (not all):
> >
> > warning: Assigned value is garbage or undefined
> > [clang-analyzer-core.uninitialized.Assign]
> > list_for_each_entry_safe(page, tmp_page, &pages, lru)
> > ^
>
> I don't think we want to disable this. Tom and others have fixed bugs
> from this report. See also:
> commit d1bd5fa07667 ("lib: remove back_str initialization")
> commit 33b5bc9e7033 ("octeontx2-af: initialize action variable")
> commit 8d783197f06d ("mctp: Fix warnings reported by clang-analyzer")
> commit eed1a5c74216 ("drm/amdgpu: check return status before using
> stable_pstate")
> commit 3da4b7403db8 ("ALSA: usb-audio: initialize variables that could
> ignore errors")
> commit 38ac2f038666 ("iio: chemical: sunrise_co2: set val parameter
> only on success")
> commit afe6949862f7 ("afs: check function return")
> commit d108370c644b ("apparmor: fix error check")
> commit d52e419ac8b5 ("rxrpc: Fix handling of an unsupported token type
> in rxrpc_read()")
> commit 6a6516c024bb ("USB: storage: avoid use of uninitialized values
> in error path")
> commit f71e41e23e12 ("iio:imu:st_lsm6dsx: check
> st_lsm6dsx_shub_read_output return")
> commit 3a61cdf43e67 ("hwrng: intel - cleanup initialization")
> commit 094dd0d73062 ("rndis_wlan: tighten check of rndis_query_oid return")
> commit e3914ed6cf44 ("ieee802154/adf7242: check status of adf7242_read_reg")
> commit c2a3d4b4cac1 ("net/mlx4_en: Cleanups suggested by clang static checker")

Thanks for the list of these commits. Will review the relevant portion of my
code to address this report.

Thank you.

Guru Das.