linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
@ 2018-10-25 19:36 Nathan Chancellor
  2018-10-25 22:20 ` Nick Desaulniers
  2018-12-17 17:41 ` Nathan Chancellor
  0 siblings, 2 replies; 6+ messages in thread
From: Nathan Chancellor @ 2018-10-25 19:36 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
  Cc: intel-gfx, dri-devel, linux-kernel, Nick Desaulniers, Nathan Chancellor

This warning is disabled by default in scripts/Makefile.extrawarn when
W= is not provided but this Makefile adds -Wall after this warning is
disabled so it shows up in the build when it shouldn't:

In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
variable 'wq' is uninitialized when used within its own initialization
[-Werror,-Wuninitialized]
        DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
                                        ^~
./include/linux/wait.h:74:63: note: expanded from macro
'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
        struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
                               ~~~~                                  ^~~~
./include/linux/wait.h:72:33: note: expanded from macro
'__WAIT_QUEUE_HEAD_INIT_ONSTACK'
        ({ init_waitqueue_head(&name); name; })
                                       ^~~~
1 error generated.

This warning looks to be a false positive given that init_waitqueue_head
initializes name before it is used. Rather than disable the warning for
the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra
clang warnings"), just disable it for the one problematic file because
it could be a useful warning for other cases.

Link: https://github.com/ClangBuiltLinux/linux/issues/220
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/gpu/drm/i915/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1c2857f13ad4..f36c420afb99 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -26,6 +26,7 @@ subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 
 # Fine grained warnings disable
 CFLAGS_i915_pci.o = $(call cc-disable-warning, override-init)
+CFLAGS_intel_breadcrumbs.o = $(call cc-disable-warning, uninitialized)
 CFLAGS_intel_fbdev.o = $(call cc-disable-warning, override-init)
 
 subdir-ccflags-y += \
-- 
2.19.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
  2018-10-25 19:36 [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o Nathan Chancellor
@ 2018-10-25 22:20 ` Nick Desaulniers
  2018-12-18 11:53   ` Chris Wilson
  2018-12-17 17:41 ` Nathan Chancellor
  1 sibling, 1 reply; 6+ messages in thread
From: Nick Desaulniers @ 2018-10-25 22:20 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: jani.nikula, joonas.lahtinen, rodrigo.vivi, intel-gfx, dri-devel,
	LKML, Matthias Kaehlcke, Chris Wilson

On Thu, Oct 25, 2018 at 12:36 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> This warning is disabled by default in scripts/Makefile.extrawarn when
> W= is not provided but this Makefile adds -Wall after this warning is
> disabled so it shows up in the build when it shouldn't:
>
> In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
> drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
> variable 'wq' is uninitialized when used within its own initialization
> [-Werror,-Wuninitialized]
>         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>                                         ^~
> ./include/linux/wait.h:74:63: note: expanded from macro
> 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
>         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
>                                ~~~~                                  ^~~~
> ./include/linux/wait.h:72:33: note: expanded from macro
> '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
>         ({ init_waitqueue_head(&name); name; })
>                                        ^~~~
> 1 error generated.
>
> This warning looks to be a false positive given that init_waitqueue_head
> initializes name before it is used. Rather than disable the warning for
> the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra

cc author/reviewer of 46e2068081e9.

I'm fine with the patch as is, unless others prefer to disable it for
the whole subdir?  We could be playing whack-a-mole in the future
disabling this warning for other translation units.

> clang warnings"), just disable it for the one problematic file because
> it could be a useful warning for other cases.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/220
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  drivers/gpu/drm/i915/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1c2857f13ad4..f36c420afb99 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -26,6 +26,7 @@ subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>
>  # Fine grained warnings disable
>  CFLAGS_i915_pci.o = $(call cc-disable-warning, override-init)
> +CFLAGS_intel_breadcrumbs.o = $(call cc-disable-warning, uninitialized)
>  CFLAGS_intel_fbdev.o = $(call cc-disable-warning, override-init)
>
>  subdir-ccflags-y += \
> --
> 2.19.1
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
  2018-10-25 19:36 [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o Nathan Chancellor
  2018-10-25 22:20 ` Nick Desaulniers
@ 2018-12-17 17:41 ` Nathan Chancellor
  1 sibling, 0 replies; 6+ messages in thread
From: Nathan Chancellor @ 2018-12-17 17:41 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
  Cc: intel-gfx, dri-devel, linux-kernel, Nick Desaulniers,
	Matthias Kaehlcke, Chris Wilson

On Thu, Oct 25, 2018 at 12:36:01PM -0700, Nathan Chancellor wrote:
> This warning is disabled by default in scripts/Makefile.extrawarn when
> W= is not provided but this Makefile adds -Wall after this warning is
> disabled so it shows up in the build when it shouldn't:
> 
> In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
> drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
> variable 'wq' is uninitialized when used within its own initialization
> [-Werror,-Wuninitialized]
>         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>                                         ^~
> ./include/linux/wait.h:74:63: note: expanded from macro
> 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
>         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
>                                ~~~~                                  ^~~~
> ./include/linux/wait.h:72:33: note: expanded from macro
> '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
>         ({ init_waitqueue_head(&name); name; })
>                                        ^~~~
> 1 error generated.
> 
> This warning looks to be a false positive given that init_waitqueue_head
> initializes name before it is used. Rather than disable the warning for
> the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra
> clang warnings"), just disable it for the one problematic file because
> it could be a useful warning for other cases.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/220
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  drivers/gpu/drm/i915/Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 1c2857f13ad4..f36c420afb99 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -26,6 +26,7 @@ subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>  
>  # Fine grained warnings disable
>  CFLAGS_i915_pci.o = $(call cc-disable-warning, override-init)
> +CFLAGS_intel_breadcrumbs.o = $(call cc-disable-warning, uninitialized)
>  CFLAGS_intel_fbdev.o = $(call cc-disable-warning, override-init)
>  
>  subdir-ccflags-y += \
> -- 
> 2.19.1
> 

Gentle ping for review.

Nathan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
  2018-10-25 22:20 ` Nick Desaulniers
@ 2018-12-18 11:53   ` Chris Wilson
  2018-12-18 19:01     ` Nathan Chancellor
  0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-12-18 11:53 UTC (permalink / raw)
  To: Nathan Chancellor, Nick Desaulniers
  Cc: jani.nikula, joonas.lahtinen, rodrigo.vivi, intel-gfx, dri-devel,
	LKML, Matthias Kaehlcke

Quoting Nick Desaulniers (2018-10-25 23:20:58)
> On Thu, Oct 25, 2018 at 12:36 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > This warning is disabled by default in scripts/Makefile.extrawarn when
> > W= is not provided but this Makefile adds -Wall after this warning is
> > disabled so it shows up in the build when it shouldn't:
> >
> > In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
> > drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
> > variable 'wq' is uninitialized when used within its own initialization
> > [-Werror,-Wuninitialized]
> >         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> >                                         ^~
> > ./include/linux/wait.h:74:63: note: expanded from macro
> > 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
> >         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> >                                ~~~~                                  ^~~~
> > ./include/linux/wait.h:72:33: note: expanded from macro
> > '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
> >         ({ init_waitqueue_head(&name); name; })
> >                                        ^~~~
> > 1 error generated.
> >
> > This warning looks to be a false positive given that init_waitqueue_head
> > initializes name before it is used. Rather than disable the warning for
> > the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra
> 
> cc author/reviewer of 46e2068081e9.
> 
> I'm fine with the patch as is, unless others prefer to disable it for
> the whole subdir?  We could be playing whack-a-mole in the future
> disabling this warning for other translation units.

Yes, exactly this since the warning is generated by a core header and a
fairly common pattern its use is not restricted to any single file.
(Will not all selftests similarly explode?)

The other false-positive clang-6 gave was for local_clock_us().
Presumably that one is fixed?
-Chris

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
  2018-12-18 11:53   ` Chris Wilson
@ 2018-12-18 19:01     ` Nathan Chancellor
  2018-12-18 19:43       ` Nick Desaulniers
  0 siblings, 1 reply; 6+ messages in thread
From: Nathan Chancellor @ 2018-12-18 19:01 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Nick Desaulniers, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, dri-devel, LKML, Matthias Kaehlcke

On Tue, Dec 18, 2018 at 11:53:06AM +0000, Chris Wilson wrote:
> Quoting Nick Desaulniers (2018-10-25 23:20:58)
> > On Thu, Oct 25, 2018 at 12:36 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > This warning is disabled by default in scripts/Makefile.extrawarn when
> > > W= is not provided but this Makefile adds -Wall after this warning is
> > > disabled so it shows up in the build when it shouldn't:
> > >
> > > In file included from drivers/gpu/drm/i915/intel_breadcrumbs.c:895:
> > > drivers/gpu/drm/i915/selftests/intel_breadcrumbs.c:350:34: error:
> > > variable 'wq' is uninitialized when used within its own initialization
> > > [-Werror,-Wuninitialized]
> > >         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> > >                                         ^~
> > > ./include/linux/wait.h:74:63: note: expanded from macro
> > > 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
> > >         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> > >                                ~~~~                                  ^~~~
> > > ./include/linux/wait.h:72:33: note: expanded from macro
> > > '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
> > >         ({ init_waitqueue_head(&name); name; })
> > >                                        ^~~~
> > > 1 error generated.
> > >
> > > This warning looks to be a false positive given that init_waitqueue_head
> > > initializes name before it is used. Rather than disable the warning for
> > > the full folder like commit 46e2068081e9 ("drm/i915: Disable some extra
> > 
> > cc author/reviewer of 46e2068081e9.
> > 
> > I'm fine with the patch as is, unless others prefer to disable it for
> > the whole subdir?  We could be playing whack-a-mole in the future
> > disabling this warning for other translation units.
> 

Hi Chris,

> Yes, exactly this since the warning is generated by a core header and a
> fairly common pattern its use is not restricted to any single file.
> (Will not all selftests similarly explode?)
> 

Well, -Wuninitialized is turned off for the whole kernel unless W= is
passed. So I suppose it should be turned back on for the whole folder
but I noticed that the i915 Makefile purposefully turns all of the
disabled warnings back on for heavier coverage so it makes some sense to
just leave it off for one translation unit when it's just one
translation unit that has the problem. That said, I'm more than happy to
send a v2 turning it off for the whole folder if you think that best.

> The other false-positive clang-6 gave was for local_clock_us().
> Presumably that one is fixed?
> -Chris

With this patch, I can build i915 using defconfig and allyesconfig
without any warnings with tip-of-tree Clang.

Thank you for the comments!
Nathan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o
  2018-12-18 19:01     ` Nathan Chancellor
@ 2018-12-18 19:43       ` Nick Desaulniers
  0 siblings, 0 replies; 6+ messages in thread
From: Nick Desaulniers @ 2018-12-18 19:43 UTC (permalink / raw)
  To: Chris Wilson
  Cc: jani.nikula, joonas.lahtinen, rodrigo.vivi, intel-gfx, dri-devel,
	LKML, Matthias Kaehlcke, Nathan Chancellor

On Tue, Dec 18, 2018 at 11:01 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
> On Tue, Dec 18, 2018 at 11:53:06AM +0000, Chris Wilson wrote:
> > The other false-positive clang-6 gave was for local_clock_us().
> > Presumably that one is fixed?
>
> With this patch, I can build i915 using defconfig and allyesconfig
> without any warnings with tip-of-tree Clang.

Also, please let us know about any other bugs found via testing with
Clang: https://github.com/ClangBuiltLinux/linux/issues

We're happy to take a look!
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-12-18 19:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 19:36 [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o Nathan Chancellor
2018-10-25 22:20 ` Nick Desaulniers
2018-12-18 11:53   ` Chris Wilson
2018-12-18 19:01     ` Nathan Chancellor
2018-12-18 19:43       ` Nick Desaulniers
2018-12-17 17:41 ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).