* [PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized
2021-08-24 22:54 [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
@ 2021-08-24 22:54 ` Nathan Chancellor
2021-08-25 9:56 ` Thomas Hellström
2021-08-24 22:54 ` [PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem() Nathan Chancellor
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-24 22:54 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Thomas Hellström, Matthew Auld,
Michael J. Ruhl, Nick Desaulniers, intel-gfx, dri-devel,
linux-kernel, clang-built-linux, llvm, Nathan Chancellor,
Dan Carpenter
Clang warns a couple of times:
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:6: warning:
variable 'import_obj' is used uninitialized whenever 'if' condition is
true [-Wsometimes-uninitialized]
if (import != &obj->base) {
^~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:80:22: note:
uninitialized use occurs here
i915_gem_object_put(import_obj);
^~~~~~~~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:2: note: remove
the 'if' if its condition is always false
if (import != &obj->base) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:38:46: note:
initialize the variable 'import_obj' to silence this warning
struct drm_i915_gem_object *obj, *import_obj;
^
= NULL
Shuffle the import_obj initialization above these if statements so that
it is not used uninitialized.
Fixes: d7b2cb380b3a ("drm/i915/gem: Correct the locking and pin pattern for dma-buf (v8)")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index ffae7df5e4d7..532c7955b300 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -59,13 +59,13 @@ static int igt_dmabuf_import_self(void *arg)
err = PTR_ERR(import);
goto out_dmabuf;
}
+ import_obj = to_intel_bo(import);
if (import != &obj->base) {
pr_err("i915_gem_prime_import created a new object!\n");
err = -EINVAL;
goto out_import;
}
- import_obj = to_intel_bo(import);
i915_gem_object_lock(import_obj, NULL);
err = __i915_gem_object_get_pages(import_obj);
@@ -176,6 +176,7 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
err = PTR_ERR(import);
goto out_dmabuf;
}
+ import_obj = to_intel_bo(import);
if (import == &obj->base) {
pr_err("i915_gem_prime_import reused gem object!\n");
@@ -183,8 +184,6 @@ static int igt_dmabuf_import_same_driver(struct drm_i915_private *i915,
goto out_import;
}
- import_obj = to_intel_bo(import);
-
i915_gem_object_lock(import_obj, NULL);
err = __i915_gem_object_get_pages(import_obj);
if (err) {
--
2.33.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized
2021-08-24 22:54 ` [PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized Nathan Chancellor
@ 2021-08-25 9:56 ` Thomas Hellström
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Hellström @ 2021-08-25 9:56 UTC (permalink / raw)
To: Nathan Chancellor, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Matthew Auld, Michael J. Ruhl, Nick Desaulniers,
intel-gfx, dri-devel, linux-kernel, clang-built-linux, llvm,
Dan Carpenter
On 8/25/21 12:54 AM, Nathan Chancellor wrote:
> Clang warns a couple of times:
>
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:6: warning:
> variable 'import_obj' is used uninitialized whenever 'if' condition is
> true [-Wsometimes-uninitialized]
> if (import != &obj->base) {
> ^~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:80:22: note:
> uninitialized use occurs here
> i915_gem_object_put(import_obj);
> ^~~~~~~~~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:63:2: note: remove
> the 'if' if its condition is always false
> if (import != &obj->base) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:38:46: note:
> initialize the variable 'import_obj' to silence this warning
> struct drm_i915_gem_object *obj, *import_obj;
> ^
> = NULL
>
> Shuffle the import_obj initialization above these if statements so that
> it is not used uninitialized.
>
> Fixes: d7b2cb380b3a ("drm/i915/gem: Correct the locking and pin pattern for dma-buf (v8)")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Patch looks good to me.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem()
2021-08-24 22:54 [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
2021-08-24 22:54 ` [PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized Nathan Chancellor
@ 2021-08-24 22:54 ` Nathan Chancellor
2021-08-25 9:57 ` Thomas Hellström
2021-08-24 22:54 ` [PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
2021-09-13 17:55 ` [PATCH 0/3] " Nathan Chancellor
3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-24 22:54 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Thomas Hellström, Matthew Auld,
Michael J. Ruhl, Nick Desaulniers, intel-gfx, dri-devel,
linux-kernel, clang-built-linux, llvm, Nathan Chancellor,
Dan Carpenter
Clang warns:
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:13: warning:
variable 'err' is used uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]
} else if (PTR_ERR(import) != -EOPNOTSUPP) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:138:9: note:
uninitialized use occurs here
return err;
^~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:9: note: remove
the 'if' if its condition is always true
} else if (PTR_ERR(import) != -EOPNOTSUPP) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:95:9: note:
initialize the variable 'err' to silence this warning
int err;
^
= 0
The test is expected to pass if i915_gem_prime_import() returns
-EOPNOTSUPP so initialize err to zero in this case.
Fixes: cdb35d1ed6d2 ("drm/i915/gem: Migrate to system at dma-buf attach time (v7)")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 532c7955b300..4a6bb64c3a35 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -128,6 +128,8 @@ static int igt_dmabuf_import_same_driver_lmem(void *arg)
pr_err("i915_gem_prime_import failed with the wrong err=%ld\n",
PTR_ERR(import));
err = PTR_ERR(import);
+ } else {
+ err = 0;
}
dma_buf_put(dmabuf);
--
2.33.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem()
2021-08-24 22:54 ` [PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem() Nathan Chancellor
@ 2021-08-25 9:57 ` Thomas Hellström
0 siblings, 0 replies; 10+ messages in thread
From: Thomas Hellström @ 2021-08-25 9:57 UTC (permalink / raw)
To: Nathan Chancellor, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Matthew Auld, Michael J. Ruhl, Nick Desaulniers,
intel-gfx, dri-devel, linux-kernel, clang-built-linux, llvm,
Dan Carpenter
On 8/25/21 12:54 AM, Nathan Chancellor wrote:
> Clang warns:
>
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:13: warning:
> variable 'err' is used uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
> } else if (PTR_ERR(import) != -EOPNOTSUPP) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:138:9: note:
> uninitialized use occurs here
> return err;
> ^~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:127:9: note: remove
> the 'if' if its condition is always true
> } else if (PTR_ERR(import) != -EOPNOTSUPP) {
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c:95:9: note:
> initialize the variable 'err' to silence this warning
> int err;
> ^
> = 0
>
> The test is expected to pass if i915_gem_prime_import() returns
> -EOPNOTSUPP so initialize err to zero in this case.
>
> Fixes: cdb35d1ed6d2 ("drm/i915/gem: Migrate to system at dma-buf attach time (v7)")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized
2021-08-24 22:54 [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
2021-08-24 22:54 ` [PATCH 1/3] drm/i915/selftests: Do not use import_obj uninitialized Nathan Chancellor
2021-08-24 22:54 ` [PATCH 2/3] drm/i915/selftests: Always initialize err in igt_dmabuf_import_same_driver_lmem() Nathan Chancellor
@ 2021-08-24 22:54 ` Nathan Chancellor
2021-09-14 15:17 ` Nick Desaulniers
2021-09-13 17:55 ` [PATCH 0/3] " Nathan Chancellor
3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-08-24 22:54 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Thomas Hellström, Matthew Auld,
Michael J. Ruhl, Nick Desaulniers, intel-gfx, dri-devel,
linux-kernel, clang-built-linux, llvm, Nathan Chancellor
This warning helps catch uninitialized variables. It should have been
enabled at the same time as commit b2423184ac33 ("drm/i915: Enable
-Wuninitialized") but I did not realize they were disabled separately.
Enable it now that i915 is clean so that it stays that way.
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/gpu/drm/i915/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 642a5b5a1b81..335ba9f43d8f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -19,7 +19,6 @@ subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
# clang warnings
subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-y += $(call cc-disable-warning, frame-address)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
--
2.33.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized
2021-08-24 22:54 ` [PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
@ 2021-09-14 15:17 ` Nick Desaulniers
0 siblings, 0 replies; 10+ messages in thread
From: Nick Desaulniers @ 2021-09-14 15:17 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Jason Ekstrand,
Thomas Hellström, Matthew Auld, Michael J. Ruhl, intel-gfx,
dri-devel, linux-kernel, clang-built-linux, llvm
On Tue, Aug 24, 2021 at 3:54 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> This warning helps catch uninitialized variables. It should have been
> enabled at the same time as commit b2423184ac33 ("drm/i915: Enable
> -Wuninitialized") but I did not realize they were disabled separately.
> Enable it now that i915 is clean so that it stays that way.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Thanks for the series!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> drivers/gpu/drm/i915/Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 642a5b5a1b81..335ba9f43d8f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -19,7 +19,6 @@ subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
> # clang warnings
> subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
> -subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
> subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
> subdir-ccflags-y += $(call cc-disable-warning, frame-address)
> subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
> --
> 2.33.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
2021-08-24 22:54 [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
` (2 preceding siblings ...)
2021-08-24 22:54 ` [PATCH 3/3] drm/i915: Enable -Wsometimes-uninitialized Nathan Chancellor
@ 2021-09-13 17:55 ` Nathan Chancellor
2021-09-14 17:10 ` Jani Nikula
3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2021-09-13 17:55 UTC (permalink / raw)
To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Thomas Hellström, Matthew Auld,
Michael J. Ruhl, Nick Desaulniers, intel-gfx, dri-devel,
linux-kernel, clang-built-linux, llvm
On Tue, Aug 24, 2021 at 03:54:24PM -0700, Nathan Chancellor wrote:
> Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
> disabled -Wsometimes-uninitialized as noisy but there have been a few
> fixes to clang that make the false positive rate fairly low so it should
> be enabled to help catch obvious mistakes. The first two patches fix
> revent instances of this warning then enables it for i915 like the rest
> of the tree.
>
> Cheers,
> Nathan
>
> Nathan Chancellor (3):
> drm/i915/selftests: Do not use import_obj uninitialized
> drm/i915/selftests: Always initialize err in
> igt_dmabuf_import_same_driver_lmem()
> drm/i915: Enable -Wsometimes-uninitialized
>
> drivers/gpu/drm/i915/Makefile | 1 -
> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ++++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
>
> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
> --
> 2.33.0
>
>
Ping, could this be picked up for an -rc as these are very clearly bugs?
Cheers,
Nathan
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
2021-09-13 17:55 ` [PATCH 0/3] " Nathan Chancellor
@ 2021-09-14 17:10 ` Jani Nikula
2021-09-14 19:53 ` Nathan Chancellor
0 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2021-09-14 17:10 UTC (permalink / raw)
To: Nathan Chancellor, Joonas Lahtinen, Rodrigo Vivi
Cc: Jason Ekstrand, Thomas Hellström, Matthew Auld,
Michael J. Ruhl, Nick Desaulniers, intel-gfx, dri-devel,
linux-kernel, clang-built-linux, llvm
On Mon, 13 Sep 2021, Nathan Chancellor <nathan@kernel.org> wrote:
> On Tue, Aug 24, 2021 at 03:54:24PM -0700, Nathan Chancellor wrote:
>> Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
>> disabled -Wsometimes-uninitialized as noisy but there have been a few
>> fixes to clang that make the false positive rate fairly low so it should
>> be enabled to help catch obvious mistakes. The first two patches fix
>> revent instances of this warning then enables it for i915 like the rest
>> of the tree.
>>
>> Cheers,
>> Nathan
>>
>> Nathan Chancellor (3):
>> drm/i915/selftests: Do not use import_obj uninitialized
>> drm/i915/selftests: Always initialize err in
>> igt_dmabuf_import_same_driver_lmem()
>> drm/i915: Enable -Wsometimes-uninitialized
>>
>> drivers/gpu/drm/i915/Makefile | 1 -
>> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ++++---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>>
>> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
>> --
>> 2.33.0
>>
>>
>
> Ping, could this be picked up for an -rc as these are very clearly bugs?
Thanks for the patches and review. Pushed to drm-intel-gt-next and
cherry-picked to drm-intel-fixes, header to -rc2.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/3] drm/i915: Enable -Wsometimes-uninitialized
2021-09-14 17:10 ` Jani Nikula
@ 2021-09-14 19:53 ` Nathan Chancellor
0 siblings, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2021-09-14 19:53 UTC (permalink / raw)
To: Jani Nikula
Cc: Joonas Lahtinen, Rodrigo Vivi, Jason Ekstrand,
Thomas Hellström, Matthew Auld, Michael J. Ruhl,
Nick Desaulniers, intel-gfx, dri-devel, linux-kernel,
clang-built-linux, llvm
On Tue, Sep 14, 2021 at 08:10:14PM +0300, Jani Nikula wrote:
> On Mon, 13 Sep 2021, Nathan Chancellor <nathan@kernel.org> wrote:
> > On Tue, Aug 24, 2021 at 03:54:24PM -0700, Nathan Chancellor wrote:
> >> Commit 46e2068081e9 ("drm/i915: Disable some extra clang warnings")
> >> disabled -Wsometimes-uninitialized as noisy but there have been a few
> >> fixes to clang that make the false positive rate fairly low so it should
> >> be enabled to help catch obvious mistakes. The first two patches fix
> >> revent instances of this warning then enables it for i915 like the rest
> >> of the tree.
> >>
> >> Cheers,
> >> Nathan
> >>
> >> Nathan Chancellor (3):
> >> drm/i915/selftests: Do not use import_obj uninitialized
> >> drm/i915/selftests: Always initialize err in
> >> igt_dmabuf_import_same_driver_lmem()
> >> drm/i915: Enable -Wsometimes-uninitialized
> >>
> >> drivers/gpu/drm/i915/Makefile | 1 -
> >> drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 7 ++++---
> >> 2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >>
> >> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
> >> --
> >> 2.33.0
> >>
> >>
> >
> > Ping, could this be picked up for an -rc as these are very clearly bugs?
>
> Thanks for the patches and review. Pushed to drm-intel-gt-next and
> cherry-picked to drm-intel-fixes, header to -rc2.
Thanks a lot!
Cheers,
Nathan
^ permalink raw reply [flat|nested] 10+ messages in thread