| Message ID | 20181025193601.11746-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Disable -Wuninitialized for intel_breadcrumbs.o | expand |
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 >
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
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
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
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!
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 += \
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(+)