| Message ID | 1528186420-6615-3-git-send-email-changbin.du@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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 -- 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
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 -- 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
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 -- 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
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 -- 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
+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.
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
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 -- 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
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.
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
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 ??
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 -- 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
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 :)
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 -- 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
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 -- 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 --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