[PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[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

Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[Nick Desaulniers]

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

Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[Chris Wilson]

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

Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[Nathan Chancellor]

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

Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[Nick Desaulniers]

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

Re: [PATCH] drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o

[Nathan Chancellor]

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