linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og)
@ 2018-05-11  8:09 changbin.du
  2018-05-11  8:09 ` [PATCH v5 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif changbin.du
                   ` (3 more replies)
  0 siblings, 4 replies; 26+ messages in thread
From: changbin.du @ 2018-05-11  8:09 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml, tglx, mingo, akpm
  Cc: rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

From: Changbin Du <changbin.du@intel.com>

Hi all,
I know some kernel developers was searching for a method to dissable GCC
optimizations, probably they want to apply GCC '-O0' option. But since Linux
kernel replys on GCC optimization to remove some dead code, so '-O0' just
breaks the build. They do need this because they want to debug kernel with
qemu, simics, kgtp or kgdb.

Thanks for the GCC '-Og' optimization level introduced in GCC 4.8, which
offers a reasonable level of optimization while maintaining fast compilation
and a good debugging experience. It is similar to '-O1' while perferring to
keep debug ability over runtime speed. With '-Og', we can build a kernel with
better debug ability and little performance drop after some simple change.

In this series, firstly introduce a new config CONFIG_NO_AUTO_INLINE after two
fixes for this new option. With this option, only functions explicitly marked
with "inline" will  be inlined. This will allow the function tracer to trace
more functions because it only traces functions that the compiler has not
inlined.

Then introduce new config CC_OPTIMIZE_FOR_DEBUGGING which apply '-Og'
optimization level for whole kernel, with a simple fix in fix_to_virt().
Currently I have only tested this option on x86 and ARM platform. Other
platforms should also work but probably need some compiling fixes as what
having done in this series. I leave that to who want to try this debug
option.

Comparison of vmlinux size: a bit smaller.

    w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ size vmlinux
       text    data     bss     dec     hex filename
    22665554   9709674  2920908 35296136        21a9388 vmlinux

    w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ size vmlinux
       text    data     bss     dec     hex filename
    21499032   10102758 2920908 34522698        20ec64a vmlinux


Comparison of system performance: a bit drop (~6%).
    This benchmark of kernel compilation is suggested by Ingo Molnar.
    https://lkml.org/lkml/2018/5/2/74

    Preparation: Set cpufreq to 'performance'.
    for ((cpu=0; cpu<120; cpu++)); do
      G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
      [ -f $G ] && echo performance > $G
    done

    w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ perf stat --repeat 5 --null --pre                 '\
        cp -a kernel ../kernel.copy.$(date +%s);         \
        rm -rf *;                                        \
        git checkout .;                                  \
        echo 1 > /proc/sys/vm/drop_caches;               \
        find ../kernel* -type f | xargs cat >/dev/null;  \
        make -j kernel >/dev/null;                       \
        make clean >/dev/null 2>&1;                      \
        sync                                            '\
                                                         \
        make -j8 >/dev/null

     Performance counter stats for 'make -j8' (5 runs):

        219.764246652 seconds time elapsed                   ( +-  0.78% )

    w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ perf stat --repeat 5 --null --pre                 '\
        cp -a kernel ../kernel.copy.$(date +%s);         \
        rm -rf *;                                        \
        git checkout .;                                  \
        echo 1 > /proc/sys/vm/drop_caches;               \
        find ../kernel* -type f | xargs cat >/dev/null;  \
        make -j kernel >/dev/null;                       \
        make clean >/dev/null 2>&1;                      \
        sync                                            '\
                                                         \
        make -j8 >/dev/null

    Performance counter stats for 'make -j8' (5 runs):

         233.574187771 seconds time elapsed                  ( +-  0.19% )

v5:
  o Exchange the position of last two patches to avoid compiling error.
v4:
  o Remove aready merged one "regulator: add dummy function of_find_regulator_by_node".

Changbin Du (4):
  x86/mm: surround level4_kernel_pgt with #ifdef
    CONFIG_X86_5LEVEL...#endif
  kernel hacking: new config NO_AUTO_INLINE to disable compiler
    auto-inline optimizations
  ARM: mm: fix build error in fix_to_virt with
    CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og
    optimization

 Makefile                          | 10 ++++++++++
 arch/arm/mm/mmu.c                 |  2 +-
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c          | 13 ++++++-------
 include/linux/compiler-gcc.h      |  2 +-
 include/linux/compiler.h          |  2 +-
 init/Kconfig                      | 19 +++++++++++++++++++
 lib/Kconfig.debug                 | 17 +++++++++++++++++
 8 files changed, 57 insertions(+), 10 deletions(-)

-- 
2.7.4

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

* [PATCH v5 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif
  2018-05-11  8:09 [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
@ 2018-05-11  8:09 ` changbin.du
  2018-05-11  8:09 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 26+ messages in thread
From: changbin.du @ 2018-05-11  8:09 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml, tglx, mingo, akpm
  Cc: rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

From: Changbin Du <changbin.du@intel.com>

The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
make code correct.

Signed-off-by: Changbin Du <changbin.du@intel.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/include/asm/pgtable_64.h |  2 ++
 arch/x86/kernel/head64.c          | 13 ++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 877bc27..9e7f667 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -15,7 +15,9 @@
 #include <linux/bitops.h>
 #include <linux/threads.h>
 
+#ifdef CONFIG_X86_5LEVEL
 extern p4d_t level4_kernel_pgt[512];
+#endif
 extern p4d_t level4_ident_pgt[512];
 extern pud_t level3_kernel_pgt[512];
 extern pud_t level3_ident_pgt[512];
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 0c408f8..775d7a6 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -143,16 +143,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
 	pgd = fixup_pointer(&early_top_pgt, physaddr);
 	p = pgd + pgd_index(__START_KERNEL_map);
-	if (la57)
-		*p = (unsigned long)level4_kernel_pgt;
-	else
-		*p = (unsigned long)level3_kernel_pgt;
-	*p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
-
+#ifdef CONFIG_X86_5LEVEL
 	if (la57) {
+		*p = (unsigned long)level4_kernel_pgt;
 		p4d = fixup_pointer(&level4_kernel_pgt, physaddr);
 		p4d[511] += load_delta;
-	}
+	} else
+#endif
+		*p = (unsigned long)level3_kernel_pgt;
+	*p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
 
 	pud = fixup_pointer(&level3_kernel_pgt, physaddr);
 	pud[510] += load_delta;
-- 
2.7.4

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

* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-05-11  8:09 [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
  2018-05-11  8:09 ` [PATCH v5 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif changbin.du
@ 2018-05-11  8:09 ` changbin.du
  2018-05-17 15:49   ` kbuild test robot
  2018-05-17 17:58   ` kbuild test robot
  2018-05-11  8:09 ` [PATCH v5 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING changbin.du
  2018-05-11  8:09 ` [PATCH v5 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization changbin.du
  3 siblings, 2 replies; 26+ messages in thread
From: changbin.du @ 2018-05-11  8:09 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml, tglx, mingo, akpm
  Cc: rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

From: Changbin Du <changbin.du@intel.com>

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Changbin Du <changbin.du@intel.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

---
v2: Some grammar updates from Steven.
---
 Makefile          |  6 ++++++
 lib/Kconfig.debug | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Makefile b/Makefile
index d0d2652..6720c40 100644
--- a/Makefile
+++ b/Makefile
@@ -775,6 +775,12 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 		   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+		   $(call cc-option, -fno-inline-small-functions) \
+		   $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifndef CC_FLAGS_FTRACE
 CC_FLAGS_FTRACE := -pg
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c40c7b7..da52243 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -198,6 +198,23 @@ config GDB_SCRIPTS
 	  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
 	  for further details.
 
+config NO_AUTO_INLINE
+	bool "Disable compiler auto-inline optimizations"
+	help
+	  This will prevent the compiler from optimizing the kernel by
+	  auto-inlining functions not marked with the inline keyword.
+	  With this option, only functions explicitly marked with
+	  "inline" will be inlined. This will allow the function tracer
+	  to trace more functions because it only traces functions that
+	  the compiler has not inlined.
+
+	  Enabling this function can help debugging a kernel if using
+	  the function tracer. But it can also change how the kernel
+	  works, because inlining functions may change the timing,
+	  which could make it difficult while debugging race conditions.
+
+	  If unsure, select N.
+
 config ENABLE_WARN_DEPRECATED
 	bool "Enable __deprecated logic"
 	default y
-- 
2.7.4

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

* [PATCH v5 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
  2018-05-11  8:09 [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
  2018-05-11  8:09 ` [PATCH v5 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif changbin.du
  2018-05-11  8:09 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
@ 2018-05-11  8:09 ` changbin.du
  2018-05-11  8:09 ` [PATCH v5 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization changbin.du
  3 siblings, 0 replies; 26+ messages in thread
From: changbin.du @ 2018-05-11  8:09 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml, tglx, mingo, akpm
  Cc: rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

From: Changbin Du <changbin.du@intel.com>

With '-Og' optimization level, GCC would not optimize a count for a loop
as a constant value. But BUILD_BUG_ON() only accept compile-time constant
values. Let's use __fix_to_virt() to avoid the error.

arch/arm/mm/mmu.o: In function `fix_to_virt':
/home/changbin/work/linux/./include/asm-generic/fixmap.h:31: undefined reference to `__compiletime_assert_31'
Makefile:1051: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Signed-off-by: Changbin Du <changbin.du@intel.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
v2: use __fix_to_virt() to fix the issue.
---
 arch/arm/mm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e46a6a4..c08d74e 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1599,7 +1599,7 @@ static void __init early_fixmap_shutdown(void)
 		pte_t *pte;
 		struct map_desc map;
 
-		map.virtual = fix_to_virt(i);
+		map.virtual = __fix_to_virt(i);
 		pte = pte_offset_early_fixmap(pmd_off_k(map.virtual), map.virtual);
 
 		/* Only i/o device mappings are supported ATM */
-- 
2.7.4

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

* [PATCH v5 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization
  2018-05-11  8:09 [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
                   ` (2 preceding siblings ...)
  2018-05-11  8:09 ` [PATCH v5 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING changbin.du
@ 2018-05-11  8:09 ` changbin.du
  3 siblings, 0 replies; 26+ messages in thread
From: changbin.du @ 2018-05-11  8:09 UTC (permalink / raw)
  To: yamada.masahiro, michal.lkml, tglx, mingo, akpm
  Cc: rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

From: Changbin Du <changbin.du@intel.com>

This will apply GCC '-Og' optimization level which is supported
since GCC 4.8. This optimization level offers a reasonable level
of optimization while maintaining fast compilation and a good
debugging experience. It is similar to '-O1' while perferring
to keep debug ability over runtime speed.

If enabling this option breaks your kernel, you should either
disable this or find a fix (mostly in the arch code). Currently
this option has only been tested on x86_64 and arm platform.

This option can satisfy people who was searching for a method
to disable compiler optimizations so to achieve better kernel
debugging experience with kgdb or qemu.

The main problem of '-Og' is we must not use __attribute__((error(msg))).
The compiler will report error though the call to error function
still can be optimize out. So we must fallback to array tricky.

Comparison of vmlinux size: a bit smaller.

    w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ size vmlinux
       text    data     bss     dec     hex filename
    22665554   9709674  2920908 35296136        21a9388 vmlinux

    w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ size vmlinux
       text    data     bss     dec     hex filename
    21499032   10102758 2920908 34522698        20ec64a vmlinux

Comparison of system performance: a bit drop (~6%).
    This benchmark of kernel compilation is suggested by Ingo Molnar.
    https://lkml.org/lkml/2018/5/2/74

    Preparation: Set cpufreq to 'performance'.
    for ((cpu=0; cpu<120; cpu++)); do
      G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
      [ -f $G ] && echo performance > $G
    done

    w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ perf stat --repeat 5 --null --pre                 '\
        cp -a kernel ../kernel.copy.$(date +%s);         \
        rm -rf *;                                        \
        git checkout .;                                  \
        echo 1 > /proc/sys/vm/drop_caches;               \
        find ../kernel* -type f | xargs cat >/dev/null;  \
        make -j kernel >/dev/null;                       \
        make clean >/dev/null 2>&1;                      \
        sync                                            '\
                                                         \
        make -j8 >/dev/null

    Performance counter stats for 'make -j8' (5 runs):

        219.764246652 seconds time elapsed                   ( +-  0.78% )

    w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
    $ perf stat --repeat 5 --null --pre                 '\
        cp -a kernel ../kernel.copy.$(date +%s);         \
        rm -rf *;                                        \
        git checkout .;                                  \
        echo 1 > /proc/sys/vm/drop_caches;               \
        find ../kernel* -type f | xargs cat >/dev/null;  \
        make -j kernel >/dev/null;                       \
        make clean >/dev/null 2>&1;                      \
        sync                                            '\
                                                         \
        make -j8 >/dev/null

    Performance counter stats for 'make -j8' (5 runs):

         233.574187771 seconds time elapsed                  ( +-  0.19% )

Signed-off-by: Changbin Du <changbin.du@intel.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
v3:
  o Rename DEBUG_EXPERIENCE to CC_OPTIMIZE_FOR_DEBUGGING
  o Move new configuration item to "General setup->Compiler optimization level"
v2:
  o Improve performance benchmark as suggested by Ingo.
  o Grammar updates in description. (Randy Dunlap)
---
 Makefile                     |  4 ++++
 include/linux/compiler-gcc.h |  2 +-
 include/linux/compiler.h     |  2 +-
 init/Kconfig                 | 19 +++++++++++++++++++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 6720c40..977418a 100644
--- a/Makefile
+++ b/Makefile
@@ -639,6 +639,9 @@ KBUILD_CFLAGS	+= $(call cc-disable-warning, format-truncation)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, format-overflow)
 KBUILD_CFLAGS	+= $(call cc-disable-warning, int-in-bool-context)
 
+ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
+KBUILD_CFLAGS	+= $(call cc-option, -Og)
+else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
 KBUILD_CFLAGS	+= $(call cc-option,-Oz,-Os)
 KBUILD_CFLAGS	+= $(call cc-disable-warning,maybe-uninitialized,)
@@ -649,6 +652,7 @@ else
 KBUILD_CFLAGS   += -O2
 endif
 endif
+endif
 
 KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
 			$(call cc-disable-warning,maybe-uninitialized,))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index b4bf73f..586ed11 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -192,7 +192,7 @@
 
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
 
-#ifndef __CHECKER__
+#if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 # define __compiletime_warning(message) __attribute__((warning(message)))
 # define __compiletime_error(message) __attribute__((error(message)))
 #endif /* __CHECKER__ */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ab4711c..e97caf4 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -301,7 +301,7 @@ unsigned long read_word_at_a_time(const void *addr)
  * sparse see a constant array size without breaking compiletime_assert on old
  * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
  */
-# ifndef __CHECKER__
+# if !defined(__CHECKER__) && !defined(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING)
 #  define __compiletime_error_fallback(condition) \
 	do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
 # endif
diff --git a/init/Kconfig b/init/Kconfig
index f013afc..aa52535 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1036,6 +1036,25 @@ config CC_OPTIMIZE_FOR_SIZE
 
 	  If unsure, say N.
 
+config CC_OPTIMIZE_FOR_DEBUGGING
+	bool "Optimize for better debugging experience (-Og)"
+	select NO_AUTO_INLINE
+	help
+	  This will apply GCC '-Og' optimization level which is supported
+	  since GCC 4.8. This optimization level offers a reasonable level
+	  of optimization while maintaining fast compilation and a good
+	  debugging experience. It is similar to '-O1' while preferring to
+	  keep debug ability over runtime speed. The overall performance
+	  will drop a bit (~6%).
+
+	  Use only if you want to debug the kernel, especially if you want
+	  to have better kernel debugging experience with gdb facilities
+	  like kgdb or qemu. If enabling this option breaks your kernel,
+	  you should either disable this or find a fix (mostly in the arch
+	  code).
+
+	  If unsure, select N.
+
 endchoice
 
 config SYSCTL
-- 
2.7.4

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-05-11  8:09 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
@ 2018-05-17 15:49   ` kbuild test robot
  2018-05-17 17:58   ` kbuild test robot
  1 sibling, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-05-17 15:49 UTC (permalink / raw)
  To: changbin.du
  Cc: kbuild-all, yamada.masahiro, michal.lkml, tglx, mingo, akpm,
	rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

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

Hi Changbin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180512-001150
config: arm64-allmodconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

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

   drivers/video/fbdev/i740fb.c: In function 'i740_calc_fifo.isra.0':
>> drivers/video/fbdev/i740fb.c:331:9: warning: 'wm' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return wm;
            ^~
--
   drivers/pci/host/pci-xgene.c: In function 'xgene_pcie_setup_ib_reg':
>> drivers/pci/host/pci-xgene.c:532:2: warning: 'pim_reg' may be used uninitialized in this function [-Wmaybe-uninitialized]
     xgene_pcie_setup_pims(port, pim_reg, pci_addr, ~(size - 1));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/wm +331 drivers/video/fbdev/i740fb.c

5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  207  
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  208  static u32 i740_calc_fifo(struct i740fb_par *par, u32 freq, int bpp)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  209  {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  210  	/*
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  211  	 * Would like to calculate these values automatically, but a generic
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  212  	 * algorithm does not seem possible.  Note: These FIFO water mark
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  213  	 * values were tested on several cards and seem to eliminate the
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  214  	 * all of the snow and vertical banding, but fine adjustments will
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  215  	 * probably be required for other cards.
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  216  	 */
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  217  
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  218  	u32 wm;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  219  
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  220  	switch (bpp) {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  221  	case 8:
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  222  		if	(freq > 200)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  223  			wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  224  		else if (freq > 175)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  225  			wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  226  		else if (freq > 135)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  227  			wm = 0x120E0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  228  		else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  229  			wm = 0x100D0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  230  		break;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  231  	case 15:
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  232  	case 16:
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  233  		if (par->has_sgram) {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  234  			if	(freq > 140)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  235  				wm = 0x2C1D0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  236  			else if (freq > 120)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  237  				wm = 0x2C180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  238  			else if (freq > 100)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  239  				wm = 0x24160000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  240  			else if (freq >  90)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  241  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  242  			else if (freq >  50)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  243  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  244  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  245  				wm = 0x13100000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  246  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  247  				wm = 0x120E0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  248  		} else {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  249  			if	(freq > 160)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  250  				wm = 0x28200000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  251  			else if (freq > 140)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  252  				wm = 0x2A1E0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  253  			else if (freq > 130)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  254  				wm = 0x2B1A0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  255  			else if (freq > 120)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  256  				wm = 0x2C180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  257  			else if (freq > 100)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  258  				wm = 0x24180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  259  			else if (freq >  90)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  260  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  261  			else if (freq >  50)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  262  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  263  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  264  				wm = 0x13100000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  265  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  266  				wm = 0x120E0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  267  		}
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  268  		break;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  269  	case 24:
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  270  		if (par->has_sgram) {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  271  			if	(freq > 130)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  272  				wm = 0x31200000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  273  			else if (freq > 120)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  274  				wm = 0x2E200000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  275  			else if (freq > 100)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  276  				wm = 0x2C1D0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  277  			else if (freq >  80)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  278  				wm = 0x25180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  279  			else if (freq >  64)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  280  				wm = 0x24160000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  281  			else if (freq >  49)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  282  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  283  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  284  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  285  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  286  				wm = 0x13100000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  287  		} else {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  288  			if	(freq > 120)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  289  				wm = 0x311F0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  290  			else if (freq > 100)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  291  				wm = 0x2C1D0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  292  			else if (freq >  80)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  293  				wm = 0x25180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  294  			else if (freq >  64)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  295  				wm = 0x24160000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  296  			else if (freq >  49)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  297  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  298  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  299  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  300  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  301  				wm = 0x13100000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  302  		}
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  303  		break;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  304  	case 32:
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  305  		if (par->has_sgram) {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  306  			if	(freq >  80)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  307  				wm = 0x2A200000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  308  			else if (freq >  60)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  309  				wm = 0x281A0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  310  			else if (freq >  49)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  311  				wm = 0x25180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  312  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  313  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  314  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  315  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  316  		} else {
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  317  			if	(freq >  80)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  318  				wm = 0x29200000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  319  			else if (freq >  60)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  320  				wm = 0x281A0000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  321  			else if (freq >  49)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  322  				wm = 0x25180000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  323  			else if (freq >  32)
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  324  				wm = 0x18120000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  325  			else
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  326  				wm = 0x16110000;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  327  		}
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  328  		break;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  329  	}
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  330  
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10 @331  	return wm;
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  332  }
5350c65f drivers/video/i740fb.c Ondrej Zary 2012-02-10  333  

:::::: The code at line 331 was first introduced by commit
:::::: 5350c65f4f15bbc111ffa629130d3f32cdd4ccf6 Resurrect Intel740 driver: i740fb

:::::: TO: Ondrej Zary <linux@rainbow-software.org>
:::::: CC: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59015 bytes --]

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-05-11  8:09 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
  2018-05-17 15:49   ` kbuild test robot
@ 2018-05-17 17:58   ` kbuild test robot
  1 sibling, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-05-17 17:58 UTC (permalink / raw)
  To: changbin.du
  Cc: kbuild-all, yamada.masahiro, michal.lkml, tglx, mingo, akpm,
	rostedt, rdunlap, x86, lgirdwood, broonie, arnd, linux-kbuild,
	linux-kernel, linux-arch, Changbin Du

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

Hi Changbin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180512-001150
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

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

   drivers//staging/comedi/drivers/pcl816.c: In function 'pcl816_ai_setup_chanlist':
>> drivers//staging/comedi/drivers/pcl816.c:171:2: warning: 'last_chan' may be used uninitialized in this function [-Wmaybe-uninitialized]
     pcl816_ai_set_chan_scan(dev, first_chan, last_chan);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers//staging/comedi/drivers/pcl818.c: In function 'pcl818_ai_setup_chanlist':
>> drivers//staging/comedi/drivers/pcl818.c:366:2: warning: 'last_chan' may be used uninitialized in this function [-Wmaybe-uninitialized]
     pcl818_ai_set_chan_scan(dev, first_chan, last_chan);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/net/wireless/ath/ath10k/mac.c: In function 'ath10k_mac_init_rd':
>> drivers/net/wireless/ath/ath10k/mac.c:8172:39: warning: 'rd' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ar->ath_common.regulatory.current_rd = rd;
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
--
   drivers/dma/stm32-mdma.c: In function 'stm32_mdma_setup_xfer':
>> drivers/dma/stm32-mdma.c:767:6: warning: 'ccr' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ccr &= ~STM32_MDMA_CCR_IRQ_MASK;
         ^~
--
   drivers/gpio/gpio-aspeed.c: In function 'enable_debounce':
>> drivers/gpio/gpio-aspeed.c:708:6: warning: 'requested_cycles' may be used uninitialized in this function [-Wmaybe-uninitialized]
      if (requested_cycles == cycles)
         ^
--
   drivers/mmc/host/sdhci-pci-core.c: In function 'intel_dsm_init.isra.3':
>> drivers/mmc/host/sdhci-pci-core.c:527:37: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized]
     intel_host->d3_retune = err ? true : !!val;
                             ~~~~~~~~~~~^~~~~~~

vim +/last_chan +171 drivers//staging/comedi/drivers/pcl816.c

19720c07 H Hartley Sweeten 2014-03-04  151  
19720c07 H Hartley Sweeten 2014-03-04  152  static void pcl816_ai_setup_chanlist(struct comedi_device *dev,
19720c07 H Hartley Sweeten 2014-03-04  153  				     unsigned int *chanlist,
19720c07 H Hartley Sweeten 2014-03-04  154  				     unsigned int seglen)
19720c07 H Hartley Sweeten 2014-03-04  155  {
19720c07 H Hartley Sweeten 2014-03-04  156  	unsigned int first_chan = CR_CHAN(chanlist[0]);
19720c07 H Hartley Sweeten 2014-03-04  157  	unsigned int last_chan;
19720c07 H Hartley Sweeten 2014-03-04  158  	unsigned int range;
19720c07 H Hartley Sweeten 2014-03-04  159  	unsigned int i;
19720c07 H Hartley Sweeten 2014-03-04  160  
19720c07 H Hartley Sweeten 2014-03-04  161  	/* store range list to card */
19720c07 H Hartley Sweeten 2014-03-04  162  	for (i = 0; i < seglen; i++) {
19720c07 H Hartley Sweeten 2014-03-04  163  		last_chan = CR_CHAN(chanlist[i]);
19720c07 H Hartley Sweeten 2014-03-04  164  		range = CR_RANGE(chanlist[i]);
19720c07 H Hartley Sweeten 2014-03-04  165  
19720c07 H Hartley Sweeten 2014-03-04  166  		pcl816_ai_set_chan_range(dev, last_chan, range);
19720c07 H Hartley Sweeten 2014-03-04  167  	}
19720c07 H Hartley Sweeten 2014-03-04  168  
19720c07 H Hartley Sweeten 2014-03-04  169  	udelay(1);
19720c07 H Hartley Sweeten 2014-03-04  170  
19720c07 H Hartley Sweeten 2014-03-04 @171  	pcl816_ai_set_chan_scan(dev, first_chan, last_chan);
19720c07 H Hartley Sweeten 2014-03-04  172  }
19720c07 H Hartley Sweeten 2014-03-04  173  

:::::: The code at line 171 was first introduced by commit
:::::: 19720c07f1f82c21311f3f7ac3e9b993598d6b70 staging: comedi: pcl816: cleanup setup_channel_list()

:::::: TO: H Hartley Sweeten <hsweeten@visionengravers.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53268 bytes --]

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-08 20:03                       ` Steven Rostedt
@ 2018-06-11 15:46                         ` Johan Hovold
  0 siblings, 0 replies; 26+ messages in thread
From: Johan Hovold @ 2018-06-11 15:46 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Johan Hovold, Viresh Kumar, Bernd Petrovitsch, Du, Changbin,
	gregkh, alex.elder, kbuild test robot, linux-arch, michal.lkml,
	linux-kernel, arnd, yamada.masahiro, lgirdwood, broonie, rdunlap,
	x86, linux, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On Fri, Jun 08, 2018 at 04:03:55PM -0400, Steven Rostedt wrote:
> On Thu, 7 Jun 2018 11:18:16 +0200
> Johan Hovold <johan@kernel.org> wrote:
> 
> 
> > If you want to work around the warning and think you can do it in some
> > non-contrived way, then go for it.
> > 
> > Clearing the request buffer, checking for termination using strnlen, and
> > then using memcpy might not be too bad.
> > 
> > But after all, it is a false positive, so leaving things as they stand
> > is fine too.
> 
> Not sure how contrived you think this is, but it solves the warning
> without adding extra work in the normal case.
> 
> -- Steve
> 
> diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
> index 71aec14f8181..4fb9f1dff47d 100644
> --- a/drivers/staging/greybus/fw-management.c
> +++ b/drivers/staging/greybus/fw-management.c
> @@ -150,15 +150,18 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
>  	}
>  
>  	request.load_method = load_method;
> -	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> +	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
>  
>  	/*
>  	 * The firmware-tag should be NULL terminated, otherwise throw error and
>  	 * fail.
>  	 */
> -	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> -		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> -		return -EINVAL;
> +	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
> +		if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> +			dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> +			return -EINVAL;
> +		}
> +		request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
>  	}

Well, I think it's quite far from obvious what is going on above, and
not least why things are being done this way (which a comment may help
with).

And just NUL-terminating the (automatic) buffer before returning wasn't
enough? Then it may be better to do away with strncpy completely.

But should we really be working around gcc this way? If the
implementation of this new warning isn't smart enough yet, should it not
just be disabled instead?

Thanks,
Johan

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  9:18                     ` Johan Hovold
  2018-06-07  9:19                       ` Viresh Kumar
@ 2018-06-08 20:03                       ` Steven Rostedt
  2018-06-11 15:46                         ` Johan Hovold
  1 sibling, 1 reply; 26+ messages in thread
From: Steven Rostedt @ 2018-06-08 20:03 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Viresh Kumar, Bernd Petrovitsch, Du, Changbin, gregkh,
	alex.elder, kbuild test robot, linux-arch, michal.lkml,
	linux-kernel, arnd, yamada.masahiro, lgirdwood, broonie, rdunlap,
	x86, linux, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On Thu, 7 Jun 2018 11:18:16 +0200
Johan Hovold <johan@kernel.org> wrote:


> If you want to work around the warning and think you can do it in some
> non-contrived way, then go for it.
> 
> Clearing the request buffer, checking for termination using strnlen, and
> then using memcpy might not be too bad.
> 
> But after all, it is a false positive, so leaving things as they stand
> is fine too.

Not sure how contrived you think this is, but it solves the warning
without adding extra work in the normal case.

-- Steve

diff --git a/drivers/staging/greybus/fw-management.c b/drivers/staging/greybus/fw-management.c
index 71aec14f8181..4fb9f1dff47d 100644
--- a/drivers/staging/greybus/fw-management.c
+++ b/drivers/staging/greybus/fw-management.c
@@ -150,15 +150,18 @@ static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
 	}
 
 	request.load_method = load_method;
-	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
+	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE - 1);
 
 	/*
 	 * The firmware-tag should be NULL terminated, otherwise throw error and
 	 * fail.
 	 */
-	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
-		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
-		return -EINVAL;
+	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 2] != '\0') {
+		if (tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
+			dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
+			return -EINVAL;
+		}
+		request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] = '\0';
 	}
 
 	/* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07 10:12                         ` Alex Elder
@ 2018-06-07 10:27                           ` Johan Hovold
  0 siblings, 0 replies; 26+ messages in thread
From: Johan Hovold @ 2018-06-07 10:27 UTC (permalink / raw)
  To: Alex Elder
  Cc: Viresh Kumar, Johan Hovold, Bernd Petrovitsch, Du, Changbin,
	Steven Rostedt, gregkh, alex.elder, kbuild test robot,
	linux-arch, michal.lkml, linux-kernel, arnd, yamada.masahiro,
	lgirdwood, broonie, rdunlap, x86, linux, linux-sparse, mingo,
	kbuild-all, akpm, changbin.du, tglx, linux-kbuild,
	linux-arm-kernel

On Thu, Jun 07, 2018 at 05:12:51AM -0500, Alex Elder wrote:
> On 06/07/2018 04:19 AM, Viresh Kumar wrote:
> > On 07-06-18, 11:18, Johan Hovold wrote:
> >> If you want to work around the warning and think you can do it in some
> >> non-contrived way, then go for it.
> >>
> >> Clearing the request buffer, checking for termination using strnlen, and
> >> then using memcpy might not be too bad.
> >>
> >> But after all, it is a false positive, so leaving things as they stand
> >> is fine too.
> > 
> > Leave it then :)
> > 
> 
> It's interesting that the warning isn't reported for this in
> fw_mgmt_interface_fw_version_operation().  The difference there is
> that you actually put a zero byte at that last position before
> returning.  I'm mildly impressed if gcc is distinguishing that.

Found a redhat blog post claiming it does check for some cases like
that:

	https://developers.redhat.com/blog/2018/05/24/detecting-string-truncation-with-gcc-8/

> You *are* returning the fw_info->firmware_tag array newly filled
> with a non-null-terminated string in one of the two cases that
> get warnings in "fw-management.c".

No, there's no warning for that one (line 250), and there fw_info is
used as the source, not the destination, so no unterminated string is
returned there either.

> But the other one is only
> updating a buffer in a local/automatic variable.

All three cases, except the one that is explicitly terminated.

> Weird.  I wish there were a non-clumsy way of marking false positives
> like this as A-OK.

The gcc docs mentions an attribute for that but it seems a bit overkill
here.

Thanks,
Johan

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  9:19                       ` Viresh Kumar
@ 2018-06-07 10:12                         ` Alex Elder
  2018-06-07 10:27                           ` Johan Hovold
  0 siblings, 1 reply; 26+ messages in thread
From: Alex Elder @ 2018-06-07 10:12 UTC (permalink / raw)
  To: Viresh Kumar, Johan Hovold
  Cc: Bernd Petrovitsch, Du, Changbin, Steven Rostedt, gregkh,
	alex.elder, kbuild test robot, linux-arch, michal.lkml,
	linux-kernel, arnd, yamada.masahiro, lgirdwood, broonie, rdunlap,
	x86, linux, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On 06/07/2018 04:19 AM, Viresh Kumar wrote:
> On 07-06-18, 11:18, Johan Hovold wrote:
>> If you want to work around the warning and think you can do it in some
>> non-contrived way, then go for it.
>>
>> Clearing the request buffer, checking for termination using strnlen, and
>> then using memcpy might not be too bad.
>>
>> But after all, it is a false positive, so leaving things as they stand
>> is fine too.
> 
> Leave it then :)
> 

It's interesting that the warning isn't reported for this in
fw_mgmt_interface_fw_version_operation().  The difference there is
that you actually put a zero byte at that last position before
returning.  I'm mildly impressed if gcc is distinguishing that.

You *are* returning the fw_info->firmware_tag array newly filled
with a non-null-terminated string in one of the two cases that
get warnings in "fw-management.c".  But the other one is only
updating a buffer in a local/automatic variable.

Weird.  I wish there were a non-clumsy way of marking false positives
like this as A-OK.

					-Alex

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  9:18                     ` Johan Hovold
@ 2018-06-07  9:19                       ` Viresh Kumar
  2018-06-07 10:12                         ` Alex Elder
  2018-06-08 20:03                       ` Steven Rostedt
  1 sibling, 1 reply; 26+ messages in thread
From: Viresh Kumar @ 2018-06-07  9:19 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Bernd Petrovitsch, Du, Changbin, Steven Rostedt, gregkh,
	alex.elder, kbuild test robot, linux-arch, michal.lkml,
	linux-kernel, arnd, yamada.masahiro, lgirdwood, broonie, rdunlap,
	x86, linux, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On 07-06-18, 11:18, Johan Hovold wrote:
> If you want to work around the warning and think you can do it in some
> non-contrived way, then go for it.
> 
> Clearing the request buffer, checking for termination using strnlen, and
> then using memcpy might not be too bad.
> 
> But after all, it is a false positive, so leaving things as they stand
> is fine too.

Leave it then :)

-- 
viresh

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  9:10                   ` Viresh Kumar
@ 2018-06-07  9:18                     ` Johan Hovold
  2018-06-07  9:19                       ` Viresh Kumar
  2018-06-08 20:03                       ` Steven Rostedt
  0 siblings, 2 replies; 26+ messages in thread
From: Johan Hovold @ 2018-06-07  9:18 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Bernd Petrovitsch, Du, Changbin, Steven Rostedt, gregkh,
	alex.elder, Johan Hovold, kbuild test robot, linux-arch,
	michal.lkml, linux-kernel, arnd, yamada.masahiro, lgirdwood,
	broonie, rdunlap, x86, linux, linux-sparse, mingo, kbuild-all,
	akpm, changbin.du, tglx, linux-kbuild, linux-arm-kernel

On Thu, Jun 07, 2018 at 02:40:25PM +0530, Viresh Kumar wrote:
> On 07-06-18, 11:03, Bernd Petrovitsch wrote:
> > On Thu, 2018-06-07 at 14:08 +0530, Viresh Kumar wrote:
> > > On 07-06-18, 15:46, Du, Changbin wrote:
> > > > I think if the destination is not a null terminated string (If I understand your
> > > > description below), memcpy can be used to get rid of such warning. The warning
> > > > makes sense in general as explained in mannual. Thanks!
> > > 
> > > The destination should be a null terminated string eventually, but we first need
> > > to make sure src is a null terminated string.
> > 
> > Is there strnlen() or memchr() in the kernel?
> > Then check the source before copying it.
> 
> It would be extra work, but memchr can be used to work around this I believe.
> 
> @Johan ??

If you want to work around the warning and think you can do it in some
non-contrived way, then go for it.

Clearing the request buffer, checking for termination using strnlen, and
then using memcpy might not be too bad.

But after all, it is a false positive, so leaving things as they stand
is fine too.

Thanks,
Johan

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  9:03                 ` Bernd Petrovitsch
@ 2018-06-07  9:10                   ` Viresh Kumar
  2018-06-07  9:18                     ` Johan Hovold
  0 siblings, 1 reply; 26+ messages in thread
From: Viresh Kumar @ 2018-06-07  9:10 UTC (permalink / raw)
  To: Bernd Petrovitsch
  Cc: Du, Changbin, Steven Rostedt, gregkh, alex.elder, Johan Hovold,
	kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	linux-sparse, mingo, kbuild-all, akpm, changbin.du, tglx,
	linux-kbuild, linux-arm-kernel

On 07-06-18, 11:03, Bernd Petrovitsch wrote:
> On Thu, 2018-06-07 at 14:08 +0530, Viresh Kumar wrote:
> > On 07-06-18, 15:46, Du, Changbin wrote:
> > > I think if the destination is not a null terminated string (If I understand your
> > > description below), memcpy can be used to get rid of such warning. The warning
> > > makes sense in general as explained in mannual. Thanks!
> > 
> > The destination should be a null terminated string eventually, but we first need
> > to make sure src is a null terminated string.
> 
> Is there strnlen() or memchr() in the kernel?
> Then check the source before copying it.

It would be extra work, but memchr can be used to work around this I believe.

@Johan ??

-- 
viresh

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  8:38               ` Viresh Kumar
@ 2018-06-07  9:03                 ` Bernd Petrovitsch
  2018-06-07  9:10                   ` Viresh Kumar
  0 siblings, 1 reply; 26+ messages in thread
From: Bernd Petrovitsch @ 2018-06-07  9:03 UTC (permalink / raw)
  To: Viresh Kumar, Du, Changbin
  Cc: Steven Rostedt, gregkh, alex.elder, Johan Hovold,
	kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	linux-sparse, mingo, kbuild-all, akpm, changbin.du, tglx,
	linux-kbuild, linux-arm-kernel

On Thu, 2018-06-07 at 14:08 +0530, Viresh Kumar wrote:
> On 07-06-18, 15:46, Du, Changbin wrote:
> > I think if the destination is not a null terminated string (If I understand your
> > description below), memcpy can be used to get rid of such warning. The warning
> > makes sense in general as explained in mannual. Thanks!
> 
> The destination should be a null terminated string eventually, but we first need
> to make sure src is a null terminated string.

Is there strnlen() or memchr() in the kernel?
Then check the source before copying it.

Kind regards,
	Bernd
-- 
Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  7:46             ` Du, Changbin
@ 2018-06-07  8:38               ` Viresh Kumar
  2018-06-07  9:03                 ` Bernd Petrovitsch
  0 siblings, 1 reply; 26+ messages in thread
From: Viresh Kumar @ 2018-06-07  8:38 UTC (permalink / raw)
  To: Du, Changbin
  Cc: Steven Rostedt, gregkh, alex.elder, Johan Hovold,
	kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	linux-sparse, mingo, kbuild-all, akpm, changbin.du, tglx,
	linux-kbuild, linux-arm-kernel

On 07-06-18, 15:46, Du, Changbin wrote:
> I think if the destination is not a null terminated string (If I understand your
> description below), memcpy can be used to get rid of such warning. The warning
> makes sense in general as explained in mannual. Thanks!

The destination should be a null terminated string eventually, but we first need
to make sure src is a null terminated string.

-- 
viresh

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  4:17           ` Viresh Kumar
  2018-06-07  7:46             ` Du, Changbin
@ 2018-06-07  8:06             ` Johan Hovold
  1 sibling, 0 replies; 26+ messages in thread
From: Johan Hovold @ 2018-06-07  8:06 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Steven Rostedt, gregkh, alex.elder, Johan Hovold,
	kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	changbin.du, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:

> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold <johan@kernel.org> wrote:
> > 
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
> 
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?

Yes, new warning in GCC 8:

	https://gcc.gnu.org/onlinedocs/gcc-8.1.0/gcc/Warning-Options.html#index-Wstringop-truncation

> Maybe I should just use memcpy here then ?

No, as you note below, you use strncpy to clear the rest of the buffer.

> But AFAIR, I used strncpy() specifically because it also sets all the remaining
> bytes after the null terminating byte with the null terminating byte. And so it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
> 
> I am not sure what would the best way to get around this incorrect-warning.

It seems gcc just isn't smart enough in this case (where you check for
overflow and never use a non-terminated string), but it is supposed to
detect when the string is unconditionally terminated. So perhaps just
adding a redundant buf[size-1] = '\0' before returning in the error path
or after the error path would shut it up. But that's a bit of a long
shot, I admit.

Probably best to leave things as they are, and let the gcc folks find a
way to handle such false positives.

Thanks,
Johan

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-07  4:17           ` Viresh Kumar
@ 2018-06-07  7:46             ` Du, Changbin
  2018-06-07  8:38               ` Viresh Kumar
  2018-06-07  8:06             ` Johan Hovold
  1 sibling, 1 reply; 26+ messages in thread
From: Du, Changbin @ 2018-06-07  7:46 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Steven Rostedt, gregkh, alex.elder, Johan Hovold,
	kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	changbin.du, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

Hi,
On Thu, Jun 07, 2018 at 09:47:18AM +0530, Viresh Kumar wrote:
> +Greg/Alex,
> 
> @Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
> body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your
> email. Bug ?
> 
> On 06-06-18, 14:26, Steven Rostedt wrote:
> > On Wed, 6 Jun 2018 16:26:00 +0200
> > Johan Hovold <johan@kernel.org> wrote:
> > 
> > > Looks like the greybus code above is working as intended by checking for
> > > unterminated string after the strncpy, even if this does now triggers
> > > the truncation warning.
> 
> So why exactly are we generating a warning here ? Is it because it is possible
> that the first n bytes of src may not have the null terminating byte and the
> dest may not be null terminated eventually ?
> 
> Maybe I should just use memcpy here then ?
> 
I think if the destination is not a null terminated string (If I understand your
description below), memcpy can be used to get rid of such warning. The warning
makes sense in general as explained in mannual. Thanks!

> But AFAIR, I used strncpy() specifically because it also sets all the remaining
> bytes after the null terminating byte with the null terminating byte. And so it
> is pretty easy for me to check if the final string is null terminated by
> checking [max - 1] byte against '\0', which the code is doing right now.
> 
> I am not sure what would the best way to get around this incorrect-warning.
> 
> And I am wondering on why buildbot reported the warning only for two instances
> in that file, while I have done the same thing at 4 places.
> 
> > Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> > should also add the "- 1" to the strncpy() so that gcc doesn't think
> > it's a mistake.
> 
> The src string is passed on from a firmware entity and we need to make sure the
> protocol (greybus) is implemented properly by the other end. For example, in the
> current case if the firmware sends "HELLOWORLD", its an error as it should have
> sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as
> "HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
> present in firmware.
> 
> -- 
> viresh

-- 
Thanks,
Changbin Du

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-06 18:26         ` Steven Rostedt
@ 2018-06-07  4:17           ` Viresh Kumar
  2018-06-07  7:46             ` Du, Changbin
  2018-06-07  8:06             ` Johan Hovold
  0 siblings, 2 replies; 26+ messages in thread
From: Viresh Kumar @ 2018-06-07  4:17 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: gregkh, alex.elder, Johan Hovold, kbuild test robot, linux-arch,
	michal.lkml, linux-kernel, arnd, yamada.masahiro, lgirdwood,
	broonie, rdunlap, x86, linux, changbin.du, linux-sparse, mingo,
	kbuild-all, akpm, changbin.du, tglx, linux-kbuild,
	linux-arm-kernel

+Greg/Alex,

@Fegguang/build-bot: I do see mention of Greg and /me in your initial email's
body saying TO: Viresh, CC: Greg, but I don't see any of us getting cc'd in your
email. Bug ?

On 06-06-18, 14:26, Steven Rostedt wrote:
> On Wed, 6 Jun 2018 16:26:00 +0200
> Johan Hovold <johan@kernel.org> wrote:
> 
> > Looks like the greybus code above is working as intended by checking for
> > unterminated string after the strncpy, even if this does now triggers
> > the truncation warning.

So why exactly are we generating a warning here ? Is it because it is possible
that the first n bytes of src may not have the null terminating byte and the
dest may not be null terminated eventually ?

Maybe I should just use memcpy here then ?

But AFAIR, I used strncpy() specifically because it also sets all the remaining
bytes after the null terminating byte with the null terminating byte. And so it
is pretty easy for me to check if the final string is null terminated by
checking [max - 1] byte against '\0', which the code is doing right now.

I am not sure what would the best way to get around this incorrect-warning.

And I am wondering on why buildbot reported the warning only for two instances
in that file, while I have done the same thing at 4 places.

> Ah, yes I now see that. Thanks for pointing it out. But perhaps it
> should also add the "- 1" to the strncpy() so that gcc doesn't think
> it's a mistake.

The src string is passed on from a firmware entity and we need to make sure the
protocol (greybus) is implemented properly by the other end. For example, in the
current case if the firmware sends "HELLOWORLD", its an error as it should have
sent "HELLWORLD\0". But with what you are saying we will forcefully make dest as
"HELLWORLD\0", which wouldn't be the right thing to do as we will miss the bug
present in firmware.

-- 
viresh

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-06 14:26       ` Johan Hovold
@ 2018-06-06 18:26         ` Steven Rostedt
  2018-06-07  4:17           ` Viresh Kumar
  0 siblings, 1 reply; 26+ messages in thread
From: Steven Rostedt @ 2018-06-06 18:26 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Viresh Kumar, kbuild test robot, linux-arch, michal.lkml,
	linux-kernel, arnd, yamada.masahiro, lgirdwood, broonie, rdunlap,
	x86, linux, changbin.du, linux-sparse, mingo, kbuild-all, akpm,
	changbin.du, tglx, linux-kbuild, linux-arm-kernel

On Wed, 6 Jun 2018 16:26:00 +0200
Johan Hovold <johan@kernel.org> wrote:

> Looks like the greybus code above is working as intended by checking for
> unterminated string after the strncpy, even if this does now triggers
> the truncation warning.

Ah, yes I now see that. Thanks for pointing it out. But perhaps it
should also add the "- 1" to the strncpy() so that gcc doesn't think
it's a mistake.

-- Steve

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-06 13:57     ` Steven Rostedt
@ 2018-06-06 14:26       ` Johan Hovold
  2018-06-06 18:26         ` Steven Rostedt
  0 siblings, 1 reply; 26+ messages in thread
From: Johan Hovold @ 2018-06-06 14:26 UTC (permalink / raw)
  To: Steven Rostedt, Viresh Kumar
  Cc: kbuild test robot, linux-arch, michal.lkml, linux-kernel, arnd,
	yamada.masahiro, lgirdwood, broonie, rdunlap, x86, linux,
	changbin.du, linux-sparse, mingo, kbuild-all, akpm, changbin.du,
	tglx, linux-kbuild, linux-arm-kernel

On Wed, Jun 06, 2018 at 09:57:14AM -0400, Steven Rostedt wrote:
> On Wed, 6 Jun 2018 05:21:55 +0800
> kbuild test robot <lkp@intel.com> wrote:
> 
> > Hi Changbin,
> > 
> > Thank you for the patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v4.17 next-20180605]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > 
> > url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
> > config: ia64-allmodconfig (attached as .config)
> > compiler: ia64-linux-gcc (GCC) 8.1.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         make.cross ARCH=ia64 
> > 
> > All warnings (new ones prefixed by >>):
> > 
> >    drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_load_and_validate_operation':
> > >> drivers//staging/greybus/fw-management.c:153:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]  
> >      strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_backend_fw_update_operation':
> >    drivers//staging/greybus/fw-management.c:304:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]
> >      strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > --
> >    drivers/auxdisplay/panel.c: In function 'panel_bind_key':
> > >> drivers/auxdisplay/panel.c:1509:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]  
> >      strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    drivers/auxdisplay/panel.c:1510:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]
> >      strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str));
> >      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Nice! This patch actually caused bugs in other areas of the code to be
> caught by the build system.
> 
> The patch is not wrong. The code that has these warnings are.

Looks like the greybus code above is working as intended by checking for
unterminated string after the strncpy, even if this does now triggers
the truncation warning.

drivers/auxdisplay/panel.c looks broken, though.

> > vim +/strncpy +153 drivers//staging/greybus/fw-management.c
> > 
> > 013e6653 Viresh Kumar 2016-05-14  138  
> > 013e6653 Viresh Kumar 2016-05-14  139  static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
> > 013e6653 Viresh Kumar 2016-05-14  140  					       u8 load_method, const char *tag)
> > 013e6653 Viresh Kumar 2016-05-14  141  {
> > 013e6653 Viresh Kumar 2016-05-14  142  	struct gb_fw_mgmt_load_and_validate_fw_request request;
> > 013e6653 Viresh Kumar 2016-05-14  143  	int ret;
> > 013e6653 Viresh Kumar 2016-05-14  144  
> > 013e6653 Viresh Kumar 2016-05-14  145  	if (load_method != GB_FW_LOAD_METHOD_UNIPRO &&
> > 013e6653 Viresh Kumar 2016-05-14  146  	    load_method != GB_FW_LOAD_METHOD_INTERNAL) {
> > 013e6653 Viresh Kumar 2016-05-14  147  		dev_err(fw_mgmt->parent,
> > 013e6653 Viresh Kumar 2016-05-14  148  			"invalid load-method (%d)\n", load_method);
> > 013e6653 Viresh Kumar 2016-05-14  149  		return -EINVAL;
> > 013e6653 Viresh Kumar 2016-05-14  150  	}
> > 013e6653 Viresh Kumar 2016-05-14  151  
> > 013e6653 Viresh Kumar 2016-05-14  152  	request.load_method = load_method;
> > b2abeaa1 Viresh Kumar 2016-08-11 @153  	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> > 013e6653 Viresh Kumar 2016-05-14  154  
> > 013e6653 Viresh Kumar 2016-05-14  155  	/*
> > 013e6653 Viresh Kumar 2016-05-14  156  	 * The firmware-tag should be NULL terminated, otherwise throw error and
> > 013e6653 Viresh Kumar 2016-05-14  157  	 * fail.
> > 013e6653 Viresh Kumar 2016-05-14  158  	 */
> > b2abeaa1 Viresh Kumar 2016-08-11  159  	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> > 013e6653 Viresh Kumar 2016-05-14  160  		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> > 013e6653 Viresh Kumar 2016-05-14  161  		return -EINVAL;
> > 013e6653 Viresh Kumar 2016-05-14  162  	}

Viresh, do you want to work around the warning somehow?

Thanks,
Johan

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-05 21:34   ` kbuild test robot
@ 2018-06-06 14:01     ` Steven Rostedt
  0 siblings, 0 replies; 26+ messages in thread
From: Steven Rostedt @ 2018-06-06 14:01 UTC (permalink / raw)
  To: kbuild test robot
  Cc: changbin.du, kbuild-all, mingo, akpm, yamada.masahiro,
	michal.lkml, tglx, rdunlap, x86, linux, lgirdwood, broonie, arnd,
	linux-kbuild, linux-kernel, linux-arch, linux-arm-kernel,
	linux-sparse, changbin.du

On Wed, 6 Jun 2018 05:34:29 +0800
kbuild test robot <lkp@intel.com> wrote:

> Hi Changbin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.17 next-20180605]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
> config: sparc64-allyesconfig (attached as .config)
> compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=sparc64 
> 
> All warnings (new ones prefixed by >>):
> 
> >> WARNING: vmlinux.o(.text.unlikely+0x1fc): Section mismatch in reference from the function init_tick_ops() to the function .init.text:get_tick_patch()  
>    The function init_tick_ops() references
>    the function __init get_tick_patch().
>    This is often because init_tick_ops lacks a __init
>    annotation or the annotation of get_tick_patch is wrong.

And again this patch uncovered a bug someplace else.

-- Steve

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

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-05 21:21   ` kbuild test robot
@ 2018-06-06 13:57     ` Steven Rostedt
  2018-06-06 14:26       ` Johan Hovold
  0 siblings, 1 reply; 26+ messages in thread
From: Steven Rostedt @ 2018-06-06 13:57 UTC (permalink / raw)
  To: kbuild test robot
  Cc: changbin.du, kbuild-all, mingo, akpm, yamada.masahiro,
	michal.lkml, tglx, rdunlap, x86, linux, lgirdwood, broonie, arnd,
	linux-kbuild, linux-kernel, linux-arch, linux-arm-kernel,
	linux-sparse, changbin.du

On Wed, 6 Jun 2018 05:21:55 +0800
kbuild test robot <lkp@intel.com> wrote:

> Hi Changbin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.17 next-20180605]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 8.1.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=ia64 
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_load_and_validate_operation':
> >> drivers//staging/greybus/fw-management.c:153:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]  
>      strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_backend_fw_update_operation':
>    drivers//staging/greybus/fw-management.c:304:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]
>      strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> --
>    drivers/auxdisplay/panel.c: In function 'panel_bind_key':
> >> drivers/auxdisplay/panel.c:1509:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]  
>      strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/auxdisplay/panel.c:1510:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]
>      strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str));
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Nice! This patch actually caused bugs in other areas of the code to be
caught by the build system.

The patch is not wrong. The code that has these warnings are.

-- Steve

> 
> vim +/strncpy +153 drivers//staging/greybus/fw-management.c
> 
> 013e6653 Viresh Kumar 2016-05-14  138  
> 013e6653 Viresh Kumar 2016-05-14  139  static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
> 013e6653 Viresh Kumar 2016-05-14  140  					       u8 load_method, const char *tag)
> 013e6653 Viresh Kumar 2016-05-14  141  {
> 013e6653 Viresh Kumar 2016-05-14  142  	struct gb_fw_mgmt_load_and_validate_fw_request request;
> 013e6653 Viresh Kumar 2016-05-14  143  	int ret;
> 013e6653 Viresh Kumar 2016-05-14  144  
> 013e6653 Viresh Kumar 2016-05-14  145  	if (load_method != GB_FW_LOAD_METHOD_UNIPRO &&
> 013e6653 Viresh Kumar 2016-05-14  146  	    load_method != GB_FW_LOAD_METHOD_INTERNAL) {
> 013e6653 Viresh Kumar 2016-05-14  147  		dev_err(fw_mgmt->parent,
> 013e6653 Viresh Kumar 2016-05-14  148  			"invalid load-method (%d)\n", load_method);
> 013e6653 Viresh Kumar 2016-05-14  149  		return -EINVAL;
> 013e6653 Viresh Kumar 2016-05-14  150  	}
> 013e6653 Viresh Kumar 2016-05-14  151  
> 013e6653 Viresh Kumar 2016-05-14  152  	request.load_method = load_method;
> b2abeaa1 Viresh Kumar 2016-08-11 @153  	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
> 013e6653 Viresh Kumar 2016-05-14  154  
> 013e6653 Viresh Kumar 2016-05-14  155  	/*
> 013e6653 Viresh Kumar 2016-05-14  156  	 * The firmware-tag should be NULL terminated, otherwise throw error and
> 013e6653 Viresh Kumar 2016-05-14  157  	 * fail.
> 013e6653 Viresh Kumar 2016-05-14  158  	 */
> b2abeaa1 Viresh Kumar 2016-08-11  159  	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
> 013e6653 Viresh Kumar 2016-05-14  160  		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
> 013e6653 Viresh Kumar 2016-05-14  161  		return -EINVAL;
> 013e6653 Viresh Kumar 2016-05-14  162  	}
> 013e6653 Viresh Kumar 2016-05-14  163  
> 013e6653 Viresh Kumar 2016-05-14  164  	/* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */
> 013e6653 Viresh Kumar 2016-05-14  165  	ret = ida_simple_get(&fw_mgmt->id_map, 1, 256, GFP_KERNEL);
> 013e6653 Viresh Kumar 2016-05-14  166  	if (ret < 0) {
> 013e6653 Viresh Kumar 2016-05-14  167  		dev_err(fw_mgmt->parent, "failed to allocate request id (%d)\n",
> 013e6653 Viresh Kumar 2016-05-14  168  			ret);
> 013e6653 Viresh Kumar 2016-05-14  169  		return ret;
> 013e6653 Viresh Kumar 2016-05-14  170  	}
> 013e6653 Viresh Kumar 2016-05-14  171  
> 013e6653 Viresh Kumar 2016-05-14  172  	fw_mgmt->intf_fw_request_id = ret;
> 04f0e6eb Viresh Kumar 2016-05-14  173  	fw_mgmt->intf_fw_loaded = false;
> 013e6653 Viresh Kumar 2016-05-14  174  	request.request_id = ret;
> 013e6653 Viresh Kumar 2016-05-14  175  
> 013e6653 Viresh Kumar 2016-05-14  176  	ret = gb_operation_sync(fw_mgmt->connection,
> 013e6653 Viresh Kumar 2016-05-14  177  				GB_FW_MGMT_TYPE_LOAD_AND_VALIDATE_FW, &request,
> 013e6653 Viresh Kumar 2016-05-14  178  				sizeof(request), NULL, 0);
> 013e6653 Viresh Kumar 2016-05-14  179  	if (ret) {
> 013e6653 Viresh Kumar 2016-05-14  180  		ida_simple_remove(&fw_mgmt->id_map,
> 013e6653 Viresh Kumar 2016-05-14  181  				  fw_mgmt->intf_fw_request_id);
> 013e6653 Viresh Kumar 2016-05-14  182  		fw_mgmt->intf_fw_request_id = 0;
> 013e6653 Viresh Kumar 2016-05-14  183  		dev_err(fw_mgmt->parent,
> 013e6653 Viresh Kumar 2016-05-14  184  			"load and validate firmware request failed (%d)\n",
> 013e6653 Viresh Kumar 2016-05-14  185  			ret);
> 013e6653 Viresh Kumar 2016-05-14  186  		return ret;
> 013e6653 Viresh Kumar 2016-05-14  187  	}
> 013e6653 Viresh Kumar 2016-05-14  188  
> 013e6653 Viresh Kumar 2016-05-14  189  	return 0;
> 013e6653 Viresh Kumar 2016-05-14  190  }
> 013e6653 Viresh Kumar 2016-05-14  191  
> 
> :::::: The code at line 153 was first introduced by commit
> :::::: b2abeaa10d5711e7730bb07120dd60ae27d7b930 greybus: firmware: s/_LEN/_SIZE
> 
> :::::: TO: Viresh Kumar <viresh.kumar@linaro.org>
> :::::: CC: Greg Kroah-Hartman <gregkh@google.com>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-05  8:13 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
  2018-06-05 21:21   ` kbuild test robot
@ 2018-06-05 21:34   ` kbuild test robot
  2018-06-06 14:01     ` Steven Rostedt
  1 sibling, 1 reply; 26+ messages in thread
From: kbuild test robot @ 2018-06-05 21:34 UTC (permalink / raw)
  To: changbin.du
  Cc: kbuild-all, mingo, akpm, yamada.masahiro, michal.lkml, rostedt,
	tglx, rdunlap, x86, linux, lgirdwood, broonie, arnd,
	linux-kbuild, linux-kernel, linux-arch, linux-arm-kernel,
	linux-sparse, changbin.du, Changbin Du

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

Hi Changbin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17 next-20180605]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text.unlikely+0x1fc): Section mismatch in reference from the function init_tick_ops() to the function .init.text:get_tick_patch()
   The function init_tick_ops() references
   the function __init get_tick_patch().
   This is often because init_tick_ops lacks a __init
   annotation or the annotation of get_tick_patch is wrong.

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53264 bytes --]

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

* Re: [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-05  8:13 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
@ 2018-06-05 21:21   ` kbuild test robot
  2018-06-06 13:57     ` Steven Rostedt
  2018-06-05 21:34   ` kbuild test robot
  1 sibling, 1 reply; 26+ messages in thread
From: kbuild test robot @ 2018-06-05 21:21 UTC (permalink / raw)
  To: changbin.du
  Cc: kbuild-all, mingo, akpm, yamada.masahiro, michal.lkml, rostedt,
	tglx, rdunlap, x86, linux, lgirdwood, broonie, arnd,
	linux-kbuild, linux-kernel, linux-arch, linux-arm-kernel,
	linux-sparse, changbin.du, Changbin Du

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

Hi Changbin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17 next-20180605]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/changbin-du-intel-com/kernel-hacking-GCC-optimization-for-better-debug-experience-Og/20180606-001415
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_load_and_validate_operation':
>> drivers//staging/greybus/fw-management.c:153:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]
     strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers//staging/greybus/fw-management.c: In function 'fw_mgmt_backend_fw_update_operation':
   drivers//staging/greybus/fw-management.c:304:2: warning: 'strncpy' specified bound 10 equals destination size [-Wstringop-truncation]
     strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/auxdisplay/panel.c: In function 'panel_bind_key':
>> drivers/auxdisplay/panel.c:1509:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]
     strncpy(key->u.kbd.press_str, press, sizeof(key->u.kbd.press_str));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/auxdisplay/panel.c:1510:2: warning: 'strncpy' specified bound 12 equals destination size [-Wstringop-truncation]
     strncpy(key->u.kbd.repeat_str, repeat, sizeof(key->u.kbd.repeat_str));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/strncpy +153 drivers//staging/greybus/fw-management.c

013e6653 Viresh Kumar 2016-05-14  138  
013e6653 Viresh Kumar 2016-05-14  139  static int fw_mgmt_load_and_validate_operation(struct fw_mgmt *fw_mgmt,
013e6653 Viresh Kumar 2016-05-14  140  					       u8 load_method, const char *tag)
013e6653 Viresh Kumar 2016-05-14  141  {
013e6653 Viresh Kumar 2016-05-14  142  	struct gb_fw_mgmt_load_and_validate_fw_request request;
013e6653 Viresh Kumar 2016-05-14  143  	int ret;
013e6653 Viresh Kumar 2016-05-14  144  
013e6653 Viresh Kumar 2016-05-14  145  	if (load_method != GB_FW_LOAD_METHOD_UNIPRO &&
013e6653 Viresh Kumar 2016-05-14  146  	    load_method != GB_FW_LOAD_METHOD_INTERNAL) {
013e6653 Viresh Kumar 2016-05-14  147  		dev_err(fw_mgmt->parent,
013e6653 Viresh Kumar 2016-05-14  148  			"invalid load-method (%d)\n", load_method);
013e6653 Viresh Kumar 2016-05-14  149  		return -EINVAL;
013e6653 Viresh Kumar 2016-05-14  150  	}
013e6653 Viresh Kumar 2016-05-14  151  
013e6653 Viresh Kumar 2016-05-14  152  	request.load_method = load_method;
b2abeaa1 Viresh Kumar 2016-08-11 @153  	strncpy(request.firmware_tag, tag, GB_FIRMWARE_TAG_MAX_SIZE);
013e6653 Viresh Kumar 2016-05-14  154  
013e6653 Viresh Kumar 2016-05-14  155  	/*
013e6653 Viresh Kumar 2016-05-14  156  	 * The firmware-tag should be NULL terminated, otherwise throw error and
013e6653 Viresh Kumar 2016-05-14  157  	 * fail.
013e6653 Viresh Kumar 2016-05-14  158  	 */
b2abeaa1 Viresh Kumar 2016-08-11  159  	if (request.firmware_tag[GB_FIRMWARE_TAG_MAX_SIZE - 1] != '\0') {
013e6653 Viresh Kumar 2016-05-14  160  		dev_err(fw_mgmt->parent, "load-and-validate: firmware-tag is not NULL terminated\n");
013e6653 Viresh Kumar 2016-05-14  161  		return -EINVAL;
013e6653 Viresh Kumar 2016-05-14  162  	}
013e6653 Viresh Kumar 2016-05-14  163  
013e6653 Viresh Kumar 2016-05-14  164  	/* Allocate ids from 1 to 255 (u8-max), 0 is an invalid id */
013e6653 Viresh Kumar 2016-05-14  165  	ret = ida_simple_get(&fw_mgmt->id_map, 1, 256, GFP_KERNEL);
013e6653 Viresh Kumar 2016-05-14  166  	if (ret < 0) {
013e6653 Viresh Kumar 2016-05-14  167  		dev_err(fw_mgmt->parent, "failed to allocate request id (%d)\n",
013e6653 Viresh Kumar 2016-05-14  168  			ret);
013e6653 Viresh Kumar 2016-05-14  169  		return ret;
013e6653 Viresh Kumar 2016-05-14  170  	}
013e6653 Viresh Kumar 2016-05-14  171  
013e6653 Viresh Kumar 2016-05-14  172  	fw_mgmt->intf_fw_request_id = ret;
04f0e6eb Viresh Kumar 2016-05-14  173  	fw_mgmt->intf_fw_loaded = false;
013e6653 Viresh Kumar 2016-05-14  174  	request.request_id = ret;
013e6653 Viresh Kumar 2016-05-14  175  
013e6653 Viresh Kumar 2016-05-14  176  	ret = gb_operation_sync(fw_mgmt->connection,
013e6653 Viresh Kumar 2016-05-14  177  				GB_FW_MGMT_TYPE_LOAD_AND_VALIDATE_FW, &request,
013e6653 Viresh Kumar 2016-05-14  178  				sizeof(request), NULL, 0);
013e6653 Viresh Kumar 2016-05-14  179  	if (ret) {
013e6653 Viresh Kumar 2016-05-14  180  		ida_simple_remove(&fw_mgmt->id_map,
013e6653 Viresh Kumar 2016-05-14  181  				  fw_mgmt->intf_fw_request_id);
013e6653 Viresh Kumar 2016-05-14  182  		fw_mgmt->intf_fw_request_id = 0;
013e6653 Viresh Kumar 2016-05-14  183  		dev_err(fw_mgmt->parent,
013e6653 Viresh Kumar 2016-05-14  184  			"load and validate firmware request failed (%d)\n",
013e6653 Viresh Kumar 2016-05-14  185  			ret);
013e6653 Viresh Kumar 2016-05-14  186  		return ret;
013e6653 Viresh Kumar 2016-05-14  187  	}
013e6653 Viresh Kumar 2016-05-14  188  
013e6653 Viresh Kumar 2016-05-14  189  	return 0;
013e6653 Viresh Kumar 2016-05-14  190  }
013e6653 Viresh Kumar 2016-05-14  191  

:::::: The code at line 153 was first introduced by commit
:::::: b2abeaa10d5711e7730bb07120dd60ae27d7b930 greybus: firmware: s/_LEN/_SIZE

:::::: TO: Viresh Kumar <viresh.kumar@linaro.org>
:::::: CC: Greg Kroah-Hartman <gregkh@google.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/gzip, Size: 49877 bytes --]

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

* [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations
  2018-06-05  8:13 [RESEND PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
@ 2018-06-05  8:13 ` changbin.du
  2018-06-05 21:21   ` kbuild test robot
  2018-06-05 21:34   ` kbuild test robot
  0 siblings, 2 replies; 26+ messages in thread
From: changbin.du @ 2018-06-05  8:13 UTC (permalink / raw)
  To: mingo, akpm, yamada.masahiro, michal.lkml
  Cc: rostedt, tglx, rdunlap, x86, linux, lgirdwood, broonie, arnd,
	linux-kbuild, linux-kernel, linux-arch, linux-arm-kernel,
	linux-sparse, changbin.du, Changbin Du

From: Changbin Du <changbin.du@intel.com>

This patch add a new kernel hacking option NO_AUTO_INLINE. Selecting
this option will prevent the compiler from optimizing the kernel by
auto-inlining functions not marked with the inline keyword.

With this option, only functions explicitly marked with "inline" will
be inlined. This will allow the function tracer to trace more functions
because it only traces functions that the compiler has not inlined.

Signed-off-by: Changbin Du <changbin.du@intel.com>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

---
v2: Some grammar updates from Steven.
---
 Makefile          |  6 ++++++
 lib/Kconfig.debug | 17 +++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Makefile b/Makefile
index d0d2652..6720c40 100644
--- a/Makefile
+++ b/Makefile
@@ -775,6 +775,12 @@ KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 		   $(call cc-option,-fno-var-tracking)
 endif
 
+ifdef CONFIG_NO_AUTO_INLINE
+KBUILD_CFLAGS   += $(call cc-option, -fno-inline-functions) \
+		   $(call cc-option, -fno-inline-small-functions) \
+		   $(call cc-option, -fno-inline-functions-called-once)
+endif
+
 ifdef CONFIG_FUNCTION_TRACER
 ifndef CC_FLAGS_FTRACE
 CC_FLAGS_FTRACE := -pg
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c40c7b7..da52243 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -198,6 +198,23 @@ config GDB_SCRIPTS
 	  instance. See Documentation/dev-tools/gdb-kernel-debugging.rst
 	  for further details.
 
+config NO_AUTO_INLINE
+	bool "Disable compiler auto-inline optimizations"
+	help
+	  This will prevent the compiler from optimizing the kernel by
+	  auto-inlining functions not marked with the inline keyword.
+	  With this option, only functions explicitly marked with
+	  "inline" will be inlined. This will allow the function tracer
+	  to trace more functions because it only traces functions that
+	  the compiler has not inlined.
+
+	  Enabling this function can help debugging a kernel if using
+	  the function tracer. But it can also change how the kernel
+	  works, because inlining functions may change the timing,
+	  which could make it difficult while debugging race conditions.
+
+	  If unsure, select N.
+
 config ENABLE_WARN_DEPRECATED
 	bool "Enable __deprecated logic"
 	default y
-- 
2.7.4

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

end of thread, other threads:[~2018-06-11 15:47 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11  8:09 [PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
2018-05-11  8:09 ` [PATCH v5 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif changbin.du
2018-05-11  8:09 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
2018-05-17 15:49   ` kbuild test robot
2018-05-17 17:58   ` kbuild test robot
2018-05-11  8:09 ` [PATCH v5 3/4] ARM: mm: fix build error in fix_to_virt with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING changbin.du
2018-05-11  8:09 ` [PATCH v5 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization changbin.du
2018-06-05  8:13 [RESEND PATCH v5 0/4] kernel hacking: GCC optimization for better debug experience (-Og) changbin.du
2018-06-05  8:13 ` [PATCH v5 2/4] kernel hacking: new config NO_AUTO_INLINE to disable compiler auto-inline optimizations changbin.du
2018-06-05 21:21   ` kbuild test robot
2018-06-06 13:57     ` Steven Rostedt
2018-06-06 14:26       ` Johan Hovold
2018-06-06 18:26         ` Steven Rostedt
2018-06-07  4:17           ` Viresh Kumar
2018-06-07  7:46             ` Du, Changbin
2018-06-07  8:38               ` Viresh Kumar
2018-06-07  9:03                 ` Bernd Petrovitsch
2018-06-07  9:10                   ` Viresh Kumar
2018-06-07  9:18                     ` Johan Hovold
2018-06-07  9:19                       ` Viresh Kumar
2018-06-07 10:12                         ` Alex Elder
2018-06-07 10:27                           ` Johan Hovold
2018-06-08 20:03                       ` Steven Rostedt
2018-06-11 15:46                         ` Johan Hovold
2018-06-07  8:06             ` Johan Hovold
2018-06-05 21:34   ` kbuild test robot
2018-06-06 14:01     ` Steven Rostedt

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