diff mbox

[v5,4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

Message ID 1528186420-6615-5-git-send-email-changbin.du@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Du, Changbin June 5, 2018, 8:13 a.m. UTC
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(-)

Comments

kernel test robot June 10, 2018, 10:44 a.m. UTC | #1
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-20180608]
[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: i386-randconfig-x079-06101602 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

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

   In file included from arch/x86/include/asm/page_32.h:35:0,
                    from arch/x86/include/asm/page.h:14,
                    from arch/x86/include/asm/thread_info.h:12,
                    from include/linux/thread_info.h:38,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:81,
                    from include/linux/spinlock.h:51,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:10,
                    from net//bluetooth/mgmt.c:27:
   net//bluetooth/mgmt.c: In function 'read_local_oob_ext_data_complete':
>> include/linux/string.h:345:9: warning: 'r256' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return __builtin_memcpy(p, q, size);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net//bluetooth/mgmt.c:5669:27: note: 'r256' was declared here
     u8 *h192, *r192, *h256, *r256;
                              ^~~~
--
   In file included from arch/x86/include/asm/page_32.h:35:0,
                    from arch/x86/include/asm/page.h:14,
                    from arch/x86/include/asm/thread_info.h:12,
                    from include/linux/thread_info.h:38,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:81,
                    from include/linux/spinlock.h:51,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:10,
                    from net/bluetooth/mgmt.c:27:
   net/bluetooth/mgmt.c: In function 'read_local_oob_ext_data_complete':
>> include/linux/string.h:345:9: warning: 'r256' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return __builtin_memcpy(p, q, size);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/bluetooth/mgmt.c:5669:27: note: 'r256' was declared here
     u8 *h192, *r192, *h256, *r256;
                              ^~~~

vim +/r256 +345 include/linux/string.h

6974f0c4 Daniel Micay 2017-07-12  332  
6974f0c4 Daniel Micay 2017-07-12  333  __FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
6974f0c4 Daniel Micay 2017-07-12  334  {
6974f0c4 Daniel Micay 2017-07-12  335  	size_t p_size = __builtin_object_size(p, 0);
6974f0c4 Daniel Micay 2017-07-12  336  	size_t q_size = __builtin_object_size(q, 0);
6974f0c4 Daniel Micay 2017-07-12  337  	if (__builtin_constant_p(size)) {
6974f0c4 Daniel Micay 2017-07-12  338  		if (p_size < size)
6974f0c4 Daniel Micay 2017-07-12  339  			__write_overflow();
6974f0c4 Daniel Micay 2017-07-12  340  		if (q_size < size)
6974f0c4 Daniel Micay 2017-07-12  341  			__read_overflow2();
6974f0c4 Daniel Micay 2017-07-12  342  	}
6974f0c4 Daniel Micay 2017-07-12  343  	if (p_size < size || q_size < size)
6974f0c4 Daniel Micay 2017-07-12  344  		fortify_panic(__func__);
6974f0c4 Daniel Micay 2017-07-12 @345  	return __builtin_memcpy(p, q, size);
6974f0c4 Daniel Micay 2017-07-12  346  }
6974f0c4 Daniel Micay 2017-07-12  347  

:::::: The code at line 345 was first introduced by commit
:::::: 6974f0c4555e285ab217cee58b6e874f776ff409 include/linux/string.h: add the option of fortified string.h functions

:::::: TO: Daniel Micay <danielmicay@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 10, 2018, 3:49 p.m. UTC | #2
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-20180608]
[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: i386-randconfig-x076-06101602 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

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//usb/typec/fusb302/fusb302.c: In function 'fusb302_handle_togdone_src':
>> drivers//usb/typec/fusb302/fusb302.c:1413:10: warning: 'ra_comp' may be used uninitialized in this function [-Wmaybe-uninitialized]
     else if (ra_comp)
             ^
--
   drivers/infiniband/ulp/ipoib/ipoib_main.c: In function 'ipoib_get_netdev':
>> drivers/infiniband/ulp/ipoib/ipoib_main.c:2021:30: warning: 'dev' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!hca->alloc_rdma_netdev || PTR_ERR(dev) == -EOPNOTSUPP)
--
   kernel//cgroup/cgroup-v1.c: In function 'cgroup1_mount':
>> kernel//cgroup/cgroup-v1.c:1268:3: warning: 'root' may be used uninitialized in this function [-Wmaybe-uninitialized]
      percpu_ref_reinit(&root->cgrp.self.refcnt);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   kernel//trace/bpf_trace.c: In function 'bpf_trace_printk':
>> kernel//trace/bpf_trace.c:226:20: warning: 'unsafe_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]
              (void *) (long) unsafe_addr,
                       ^~~~~~~~~~~~~~~~~~
   kernel//trace/bpf_trace.c:170:6: note: 'unsafe_addr' was declared here
     u64 unsafe_addr;
         ^~~~~~~~~~~
--
   net//6lowpan/iphc.c: In function 'lowpan_header_decompress':
   net//6lowpan/iphc.c:617:12: warning: 'iphc1' may be used uninitialized in this function [-Wmaybe-uninitialized]
     u8 iphc0, iphc1, cid = 0;
               ^~~~~
>> net//6lowpan/iphc.c:617:5: warning: 'iphc0' may be used uninitialized in this function [-Wmaybe-uninitialized]
     u8 iphc0, iphc1, cid = 0;
        ^~~~~
--
   net//netfilter/nf_tables_api.c: In function 'nf_tables_dump_set':
>> net//netfilter/nf_tables_api.c:3625:2: warning: 'set' may be used uninitialized in this function [-Wmaybe-uninitialized]
     set->ops->walk(&dump_ctx->ctx, set, &args.iter);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/media/dvb-frontends/mn88472.c: In function 'mn88472_set_frontend':
>> drivers/media/dvb-frontends/mn88472.c:339:27: warning: 'bandwidth_vals_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]
            bandwidth_vals_ptr[i]);
                              ^
>> drivers/media/dvb-frontends/mn88472.c:320:8: warning: 'bandwidth_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
     ret = regmap_write(dev->regmap[2], 0x04, bandwidth_val);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/media/dvb-frontends/mn88473.c: In function 'mn88473_set_frontend':
>> drivers/media/dvb-frontends/mn88473.c:162:7: warning: 'conf_val_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]
      ret = regmap_bulk_write(dev->regmap[1], 0x10, conf_val_ptr, 6);
      ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   net//netfilter/ipvs/ip_vs_sync.c: In function 'ip_vs_sync_conn':
>> net//netfilter/ipvs/ip_vs_sync.c:731:13: warning: 'm' may be used uninitialized in this function [-Wmaybe-uninitialized]
     m->nr_conns++;
     ~~~~~~~~~~~^~
--
   drivers//hwspinlock/hwspinlock_core.c: In function 'of_hwspin_lock_get_id':
>> drivers//hwspinlock/hwspinlock_core.c:339:19: warning: 'id' may be used uninitialized in this function [-Wmaybe-uninitialized]
     return ret ? ret : id;
            ~~~~~~~~~~^~~~
--
   drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_nexthop_group_update':
>> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:3078:7: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
       if (err)
          ^

vim +/ra_comp +1413 drivers//usb/typec/fusb302/fusb302.c

c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1359  
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1360  static int fusb302_handle_togdone_src(struct fusb302_chip *chip,
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1361  				      u8 togdone_result)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1362  {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1363  	/*
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1364  	 * - set polarity (measure cc, vconn, tx)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1365  	 * - set pull_up, pull_down
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1366  	 * - set cc1, cc2, and update to tcpm_port
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1367  	 * - set I_COMP interrupt on
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1368  	 */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1369  	int ret = 0;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1370  	u8 status0;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1371  	u8 ra_mda = ra_mda_value[chip->src_current_status];
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1372  	u8 rd_mda = rd_mda_value[chip->src_current_status];
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1373  	bool ra_comp, rd_comp;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1374  	enum typec_cc_polarity cc_polarity;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1375  	enum typec_cc_status cc_status_active, cc1, cc2;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1376  
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1377  	/* set pull_up, pull_down */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1378  	ret = fusb302_set_cc_pull(chip, true, false);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1379  	if (ret < 0) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1380  		fusb302_log(chip, "cannot set cc to pull up, ret=%d", ret);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1381  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1382  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1383  	/* set polarity */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1384  	cc_polarity = (togdone_result == FUSB_REG_STATUS1A_TOGSS_SRC1) ?
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1385  		      TYPEC_POLARITY_CC1 : TYPEC_POLARITY_CC2;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1386  	ret = fusb302_set_cc_polarity(chip, cc_polarity);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1387  	if (ret < 0) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1388  		fusb302_log(chip, "cannot set cc polarity %s, ret=%d",
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1389  			    cc_polarity_name[cc_polarity], ret);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1390  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1391  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1392  	/* fusb302_set_cc_polarity() has set the correct measure block */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1393  	ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1394  	if (ret < 0)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1395  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1396  	usleep_range(50, 100);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1397  	ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1398  	if (ret < 0)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1399  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1400  	rd_comp = !!(status0 & FUSB_REG_STATUS0_COMP);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1401  	if (!rd_comp) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1402  		ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, ra_mda);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1403  		if (ret < 0)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1404  			return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1405  		usleep_range(50, 100);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1406  		ret = fusb302_i2c_read(chip, FUSB_REG_STATUS0, &status0);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1407  		if (ret < 0)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1408  			return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1409  		ra_comp = !!(status0 & FUSB_REG_STATUS0_COMP);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1410  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1411  	if (rd_comp)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1412  		cc_status_active = TYPEC_CC_OPEN;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27 @1413  	else if (ra_comp)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1414  		cc_status_active = TYPEC_CC_RD;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1415  	else
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1416  		/* Ra is not supported, report as Open */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1417  		cc_status_active = TYPEC_CC_OPEN;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1418  	/* restart toggling if the cc status on the active line is OPEN */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1419  	if (cc_status_active == TYPEC_CC_OPEN) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1420  		fusb302_log(chip, "restart toggling as CC_OPEN detected");
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1421  		ret = fusb302_set_toggling(chip, chip->toggling_mode);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1422  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1423  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1424  	/* update tcpm with the new cc value */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1425  	cc1 = (cc_polarity == TYPEC_POLARITY_CC1) ?
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1426  	      cc_status_active : TYPEC_CC_OPEN;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1427  	cc2 = (cc_polarity == TYPEC_POLARITY_CC2) ?
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1428  	      cc_status_active : TYPEC_CC_OPEN;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1429  	if ((chip->cc1 != cc1) || (chip->cc2 != cc2)) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1430  		chip->cc1 = cc1;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1431  		chip->cc2 = cc2;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1432  		tcpm_cc_change(chip->tcpm_port);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1433  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1434  	/* turn off toggling */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1435  	ret = fusb302_set_toggling(chip, TOGGLINE_MODE_OFF);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1436  	if (ret < 0) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1437  		fusb302_log(chip,
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1438  			    "cannot set toggling mode off, ret=%d", ret);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1439  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1440  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1441  	/* set MDAC to Rd threshold, and unmask I_COMP for unplug detection */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1442  	ret = fusb302_i2c_write(chip, FUSB_REG_MEASURE, rd_mda);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1443  	if (ret < 0)
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1444  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1445  	/* unmask comp_chng interrupt */
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1446  	ret = fusb302_i2c_clear_bits(chip, FUSB_REG_MASK,
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1447  				     FUSB_REG_MASK_COMP_CHNG);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1448  	if (ret < 0) {
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1449  		fusb302_log(chip,
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1450  			    "cannot unmask bc_lcl interrupt, ret=%d", ret);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1451  		return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1452  	}
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1453  	chip->intr_comp_chng = true;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1454  	fusb302_log(chip, "detected cc1=%s, cc2=%s",
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1455  		    typec_cc_status_name[cc1],
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1456  		    typec_cc_status_name[cc2]);
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1457  
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1458  	return ret;
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1459  }
c034a43e drivers/staging/typec/fusb302/fusb302.c Yueyao Zhu 2017-04-27  1460  

:::::: The code at line 1413 was first introduced by commit
:::::: c034a43e72dda58e4a184d71f5502ef356e04453 staging: typec: Fairchild FUSB302 Type-c chip driver

:::::: TO: Yueyao Zhu <yueyao@google.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
Steven Rostedt June 11, 2018, 4 p.m. UTC | #3
On Sun, 10 Jun 2018 23:49: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-20180608]
> [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: i386-randconfig-x076-06101602 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> 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//usb/typec/fusb302/fusb302.c: In function 'fusb302_handle_togdone_src':
> >> drivers//usb/typec/fusb302/fusb302.c:1413:10: warning: 'ra_comp' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      else if (ra_comp)
>              ^

This is a false warning. I'm surprised gcc couldn't catch it. Although
that code looks like it could have been  done a bit nicer.


> --
>    drivers/infiniband/ulp/ipoib/ipoib_main.c: In function 'ipoib_get_netdev':
> >> drivers/infiniband/ulp/ipoib/ipoib_main.c:2021:30: warning: 'dev' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      if (!hca->alloc_rdma_netdev || PTR_ERR(dev) == -EOPNOTSUPP)
> --

Strange, this is also false, with the same construct.

	if (a) {
		b = init;
	}
	if (!a) {
		use b;

It warns that b may be unused. I'm guessing the extra option we add in
gcc by the patch causes gcc to break in this regard.



>    kernel//cgroup/cgroup-v1.c: In function 'cgroup1_mount':
> >> kernel//cgroup/cgroup-v1.c:1268:3: warning: 'root' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>       percpu_ref_reinit(&root->cgrp.self.refcnt);
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> --

Slightly different construct, but similar:

	ret = func();
	if (ret)
		goto out_unlock;

	root = init;

 out_unlock:

	if (ret)
		return;

	use root;



>    kernel//trace/bpf_trace.c: In function 'bpf_trace_printk':
> >> kernel//trace/bpf_trace.c:226:20: warning: 'unsafe_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>               (void *) (long) unsafe_addr,
>                        ^~~~~~~~~~~~~~~~~~

Again similar:

	if (fmt_cnt >= 3)
		return;

	switch (fmt_cnt) {
	case 1:
		unsafe_addr = init;
		break;
	case 2:
		unsafe_addr = init2;
		break;
	case 3:
		unsafe_addr = init3;
		break;
	}

	use init;


>    kernel//trace/bpf_trace.c:170:6: note: 'unsafe_addr' was declared here
>      u64 unsafe_addr;
>          ^~~~~~~~~~~
> --
>    net//6lowpan/iphc.c: In function 'lowpan_header_decompress':
>    net//6lowpan/iphc.c:617:12: warning: 'iphc1' may be used uninitialized in this function [-Wmaybe-uninitialized]
>      u8 iphc0, iphc1, cid = 0;
>                ^~~~~
> >> net//6lowpan/iphc.c:617:5: warning: 'iphc0' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      u8 iphc0, iphc1, cid = 0;
>         ^~~~~

Similar but crazier:

	if (lowpan_fetch_skb(&iphc0) ||
	    lowpan_fetch_skb(&iphc1))
		return;

	use iphc0 and ipch1;

where lowpan_fetch_skb() is:

	if (test())
		return true;

	init data (iphc0 or iphc1);
	return false;


> --
>    net//netfilter/nf_tables_api.c: In function 'nf_tables_dump_set':
> >> net//netfilter/nf_tables_api.c:3625:2: warning: 'set' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      set->ops->walk(&dump_ctx->ctx, set, &args.iter);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I don't have the same kernel, as this doesn't match. But I'm sure it's
a false positive like the others.


> --
>    drivers/media/dvb-frontends/mn88472.c: In function 'mn88472_set_frontend':
> >> drivers/media/dvb-frontends/mn88472.c:339:27: warning: 'bandwidth_vals_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>             bandwidth_vals_ptr[i]);
>                               ^
> >> drivers/media/dvb-frontends/mn88472.c:320:8: warning: 'bandwidth_val' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      ret = regmap_write(dev->regmap[2], 0x04, bandwidth_val);
>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one may not be a false positive. It really looks like there's a
path to that being used uninitialized. But I haven't torn that function
apart enough to really tell, but I don't fault gcc for not warning
about it. But I like to know if gcc doesn't warn without this patch?


> --
>    drivers/media/dvb-frontends/mn88473.c: In function 'mn88473_set_frontend':
> >> drivers/media/dvb-frontends/mn88473.c:162:7: warning: 'conf_val_ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>       ret = regmap_bulk_write(dev->regmap[1], 0x10, conf_val_ptr, 6);
>       ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Same as the one before it. Need to see if this isn't really a real
issue.

> --
>    net//netfilter/ipvs/ip_vs_sync.c: In function 'ip_vs_sync_conn':
> >> net//netfilter/ipvs/ip_vs_sync.c:731:13: warning: 'm' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      m->nr_conns++;
>      ~~~~~~~~~~~^~

gcc is really stupid on this one.

	if (buff)
		init m;
	if (!buff)
		init m;

	use m;

Really?

> --
>    drivers//hwspinlock/hwspinlock_core.c: In function 'of_hwspin_lock_get_id':
> >> drivers//hwspinlock/hwspinlock_core.c:339:19: warning: 'id' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>      return ret ? ret : id;
>             ~~~~~~~~~~^~~~


Again, we jump here without initializing 'id' when ret is set.


> --
>    drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_nexthop_group_update':
> >> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:3078:7: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]  
>        if (err)
>           ^
> 
> vim +/ra_comp +1413 drivers//usb/typec/fusb302/fusb302.c
> 


Another switch statement false positive:

	nh->type can only be set to two different values, and then we
	have:

	switch (nh->type) {
	case value1:
		err = func();
		break;
	case value2:
		err = func2();
		break;
	}
	if (err)


Of all the warnings, only one looks like it could be a possible issue.
Thus, this patch causes gcc to fail more on it analysis. The one
possible issue should have been caught by gcc without this patch, so
I'm skeptical that it is indeed an issue, but it's complex and I am
impressed if gcc really did figure it out.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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