[PATCH 1/2] scripts/clang-tools: Create array of checks to exclude

[PATCH 1/2] scripts/clang-tools: Create array of checks to exclude

[Guru Das Srinagesh]

Create array of clang-analyzer checks to exclude for ease of adding
more. This is per the suggestion in an earlier discussion [1].

[1] https://lore.kernel.org/lkml/CAKwvOdnbMs-pLRfo4O-MHOF=9=kAvDuktkeeeX7bkmnLi8LWnw@mail.gmail.com/

Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
---
 scripts/clang-tools/run-clang-tools.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
index bb78c9b..a72c4c7 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -50,7 +50,12 @@ def run_analysis(entry):
         checks += "linuxkernel-*"
     else:
         checks += "clang-analyzer-*"
-        checks += ",-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling"
+
+        # List of checks to be excluded
+        exclude = []
+        exclude.append("-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
+
+        checks += ''.join(["," + e for e in exclude])
     p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
                        stdout=subprocess.PIPE,
                        stderr=subprocess.STDOUT,
-- 
2.7.4

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

[Guru Das Srinagesh]

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 <quic_gurus@quicinc.com>
---
 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

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

[Nick Desaulniers]

On Wed, Oct 5, 2022 at 8:24 PM Guru Das Srinagesh
<quic_gurus@quicinc.com> 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 <quic_gurus@quicinc.com>
> ---
>  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

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

[Guru Das Srinagesh]

On Oct 10 2022 10:22, Nick Desaulniers wrote:
> On Wed, Oct 5, 2022 at 8:24 PM Guru Das Srinagesh
> <quic_gurus@quicinc.com> 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.

Re: [PATCH 1/2] scripts/clang-tools: Create array of checks to exclude

[Nick Desaulniers]

On Wed, Oct 5, 2022 at 8:24 PM Guru Das Srinagesh
<quic_gurus@quicinc.com> wrote:
>
> Create array of clang-analyzer checks to exclude for ease of adding
> more. This is per the suggestion in an earlier discussion [1].
>
> [1] https://lore.kernel.org/lkml/CAKwvOdnbMs-pLRfo4O-MHOF=9=kAvDuktkeeeX7bkmnLi8LWnw@mail.gmail.com/
>
> Signed-off-by: Guru Das Srinagesh <quic_gurus@quicinc.com>
> ---
>  scripts/clang-tools/run-clang-tools.py | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
> index bb78c9b..a72c4c7 100755
> --- a/scripts/clang-tools/run-clang-tools.py
> +++ b/scripts/clang-tools/run-clang-tools.py
> @@ -50,7 +50,12 @@ def run_analysis(entry):
>          checks += "linuxkernel-*"
>      else:
>          checks += "clang-analyzer-*"
> -        checks += ",-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling"
> +
> +        # List of checks to be excluded
> +        exclude = []
> +        exclude.append("-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling")
> +
> +        checks += ''.join(["," + e for e in exclude])

Thanks for the patch.  I think it's more canonical Python to do:

checks += ",".join(exclude)

However, let's just make `checks` a list to start with? Then join it
at the very end?

Even for clang-tidy, we would have a comma separated list:
`"-check=-*,linuxkernel-*"`

>      p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],

^ so then this could be:

p = subprocess.run(["clang-tidy", "-p", args.path, ",".join(checks), ...

>                         stdout=subprocess.PIPE,
>                         stderr=subprocess.STDOUT,
> --
> 2.7.4
>


-- 
Thanks,
~Nick Desaulniers