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
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) {
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
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);
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
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
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
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
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.
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
dri-devel@lists.freedesktop.org