linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* sound/soc/codecs/wm8962.c:2790:32: warning: 'fratio' may be used uninitialized in this function
@ 2016-01-25  7:15 kbuild test robot
  2016-01-25 16:01 ` [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives Andrey Ryabinin
  0 siblings, 1 reply; 4+ messages in thread
From: kbuild test robot @ 2016-01-25  7:15 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: kbuild-all, linux-kernel, Andrew Morton,
	Linux Memory Management List

[-- Attachment #1: Type: text/plain, Size: 3250 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   92e963f50fc74041b5e9e744c330dca48e04f08d
commit: c6d308534aef6c99904bf5862066360ae067abc4 UBSAN: run-time undefined behavior sanity checker
date:   4 days ago
config: x86_64-randconfig-n0-01251456 (attached as .config)
reproduce:
        git checkout c6d308534aef6c99904bf5862066360ae067abc4
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   sound/soc/codecs/wm8962.c: In function 'wm8962_set_fll':
>> sound/soc/codecs/wm8962.c:2790:32: warning: 'fratio' may be used uninitialized in this function [-Wmaybe-uninitialized]
     fll_div->n = target / (fratio * Fref);
                                   ^
   sound/soc/codecs/wm8962.c:2740:15: note: 'fratio' was declared here
     unsigned int fratio, gcd_fll;
                  ^

vim +/fratio +2790 sound/soc/codecs/wm8962.c

9a76f1ff Mark Brown      2010-08-05  2774  
9a76f1ff Mark Brown      2010-08-05  2775  	pr_debug("FLL Fvco=%dHz\n", target);
9a76f1ff Mark Brown      2010-08-05  2776  
25985edc Lucas De Marchi 2011-03-30  2777  	/* Find an appropriate FLL_FRATIO and factor it out of the target */
9a76f1ff Mark Brown      2010-08-05  2778  	for (i = 0; i < ARRAY_SIZE(fll_fratios); i++) {
9a76f1ff Mark Brown      2010-08-05  2779  		if (fll_fratios[i].min <= Fref && Fref <= fll_fratios[i].max) {
9a76f1ff Mark Brown      2010-08-05  2780  			fll_div->fll_fratio = fll_fratios[i].fll_fratio;
9a76f1ff Mark Brown      2010-08-05  2781  			fratio = fll_fratios[i].ratio;
9a76f1ff Mark Brown      2010-08-05  2782  			break;
9a76f1ff Mark Brown      2010-08-05  2783  		}
9a76f1ff Mark Brown      2010-08-05  2784  	}
9a76f1ff Mark Brown      2010-08-05  2785  	if (i == ARRAY_SIZE(fll_fratios)) {
9a76f1ff Mark Brown      2010-08-05  2786  		pr_err("Unable to find FLL_FRATIO for Fref=%uHz\n", Fref);
9a76f1ff Mark Brown      2010-08-05  2787  		return -EINVAL;
9a76f1ff Mark Brown      2010-08-05  2788  	}
9a76f1ff Mark Brown      2010-08-05  2789  
9a76f1ff Mark Brown      2010-08-05 @2790  	fll_div->n = target / (fratio * Fref);
9a76f1ff Mark Brown      2010-08-05  2791  
9a76f1ff Mark Brown      2010-08-05  2792  	if (target % Fref == 0) {
9a76f1ff Mark Brown      2010-08-05  2793  		fll_div->theta = 0;
9a76f1ff Mark Brown      2010-08-05  2794  		fll_div->lambda = 0;
9a76f1ff Mark Brown      2010-08-05  2795  	} else {
9a76f1ff Mark Brown      2010-08-05  2796  		gcd_fll = gcd(target, fratio * Fref);
9a76f1ff Mark Brown      2010-08-05  2797  
9a76f1ff Mark Brown      2010-08-05  2798  		fll_div->theta = (target - (fll_div->n * fratio * Fref))

:::::: The code at line 2790 was first introduced by commit
:::::: 9a76f1ff6e299fbb04149fe15aff061351fd0dab ASoC: Add initial WM8962 CODEC driver

:::::: TO: Mark Brown <broonie@opensource.wolfsonmicro.com>
:::::: CC: Mark Brown <broonie@opensource.wolfsonmicro.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24943 bytes --]

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

* [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives
  2016-01-25  7:15 sound/soc/codecs/wm8962.c:2790:32: warning: 'fratio' may be used uninitialized in this function kbuild test robot
@ 2016-01-25 16:01 ` Andrey Ryabinin
  2016-01-25 21:41   ` Andrew Morton
  0 siblings, 1 reply; 4+ messages in thread
From: Andrey Ryabinin @ 2016-01-25 16:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: fengguang.wu, linux-kernel, kbuild-all, Andrey Ryabinin

-fsanitize=* options makes GCC less smart than usual and increase number
of 'maybe-uninitialized' false-positives. So this patch does two things:
 * Add -Wno-maybe-uninitialized to CFLAGS_UBSAN which will disable all
   such warnings for instrumented files.
 * Remove CONFIG_UBSAN_SANITIZE_ALL from all[yes|mod]config builds. So
   the all[yes|mod]config build goes without -fsanitize=* and still with
   -Wmaybe-uninitialized.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
 lib/Kconfig.ubsan      | 5 +++++
 scripts/Makefile.ubsan | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index 49518fb..82da62bb 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -13,6 +13,11 @@ config UBSAN_SANITIZE_ALL
 	bool "Enable instrumentation for the entire kernel"
 	depends on UBSAN
 	depends on ARCH_HAS_UBSAN_SANITIZE_ALL
+
+	# We build with -Wno-maybe-uninitilzed, but we still want to
+	# use -Wmaybe-uninitilized in allmodconfig builds.
+	# So dependsy bellow used to disable this option in allmodconfig
+	depends on !COMPILE_TEST
 	default y
 	help
 	  This option activates instrumentation for the entire kernel.
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index 8ab6867..77ce538 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -14,4 +14,8 @@ ifdef CONFIG_UBSAN
 ifdef CONFIG_UBSAN_ALIGNMENT
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=alignment)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
-- 
2.4.10

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

* Re: [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives
  2016-01-25 16:01 ` [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives Andrey Ryabinin
@ 2016-01-25 21:41   ` Andrew Morton
  2016-01-26 17:03     ` Andrey Ryabinin
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2016-01-25 21:41 UTC (permalink / raw)
  To: Andrey Ryabinin; +Cc: fengguang.wu, linux-kernel, kbuild-all

On Mon, 25 Jan 2016 19:01:34 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:

> -fsanitize=* options makes GCC less smart than usual and increase number
> of 'maybe-uninitialized' false-positives. So this patch does two things:
>  * Add -Wno-maybe-uninitialized to CFLAGS_UBSAN which will disable all
>    such warnings for instrumented files.
>  * Remove CONFIG_UBSAN_SANITIZE_ALL from all[yes|mod]config builds. So
>    the all[yes|mod]config build goes without -fsanitize=* and still with
>    -Wmaybe-uninitialized.

hm, that's a bit sad.

We have no means of working out whether we should re-enable
maybe-uninitialized for later gcc's, as they become smarter about this.
What do we do, just "remember" to try it later on?

Do you know if this issue is on the gcc developer' radar?

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

* Re: [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives
  2016-01-25 21:41   ` Andrew Morton
@ 2016-01-26 17:03     ` Andrey Ryabinin
  0 siblings, 0 replies; 4+ messages in thread
From: Andrey Ryabinin @ 2016-01-26 17:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: fengguang.wu, linux-kernel, kbuild-all

On 01/26/2016 12:41 AM, Andrew Morton wrote:
> On Mon, 25 Jan 2016 19:01:34 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> 
>> -fsanitize=* options makes GCC less smart than usual and increase number
>> of 'maybe-uninitialized' false-positives. So this patch does two things:
>>  * Add -Wno-maybe-uninitialized to CFLAGS_UBSAN which will disable all
>>    such warnings for instrumented files.
>>  * Remove CONFIG_UBSAN_SANITIZE_ALL from all[yes|mod]config builds. So
>>    the all[yes|mod]config build goes without -fsanitize=* and still with
>>    -Wmaybe-uninitialized.
> 
> hm, that's a bit sad.
> 
> We have no means of working out whether we should re-enable
> maybe-uninitialized for later gcc's, as they become smarter about this.
> What do we do, just "remember" to try it later on?
> 

I don't see anything bad about it. Note, that CONFIG_UBSAN_SANITIZE_ALL=y *only* adds
-fsanitize=* to CFLAGS and this patch removes only CONFIG_UBSAN_SANITIZE_ALL from allyesconfig, but not the CONFIG_UBSAN.

So now, we do allyesconfig build without CONFIG_UBSAN_SANITIZE_ALL (iow without -fsantize=*), but still with CONFIG_UBSAN=y.
Which means that we still build lib/ubsan.c (and with -Wmaybe-uninitialized).

> Do you know if this issue is on the gcc developer' radar?
> 

I don't know, but it's unlikely that something will be changed here. -Wmaybe-uninitialized will always be prone to
false-positives, simply by definition of it(if GCC could prove that variable is uninitialized it will issue another
warning -Wuninitialized). And since -fsanitize=* causes significant changes in generated code, the influence on
-Wmaybe-uninitialized likely will retain.

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

end of thread, other threads:[~2016-01-26 17:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-25  7:15 sound/soc/codecs/wm8962.c:2790:32: warning: 'fratio' may be used uninitialized in this function kbuild test robot
2016-01-25 16:01 ` [PATCH] ubsan: fix tree-wide -Wmaybe-uninitialized false positives Andrey Ryabinin
2016-01-25 21:41   ` Andrew Morton
2016-01-26 17:03     ` Andrey Ryabinin

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).