| Message ID | 20200918103430.297167-4-thuth@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Update Travis from Xenial to Bionic and Focal | expand |
On Fri, Sep 18, 2020 at 12:34:27PM +0200, Thomas Huth wrote: > GCC 9.3.0 on Ubuntu complains: > > In file included from /usr/include/string.h:495, > from /home/travis/build/huth/qemu/include/qemu/osdep.h:87, > from ../migration/global_state.c:13: > In function ‘strncpy’, > inlined from ‘global_state_store_running’ at ../migration/global_state.c:47:5: > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: > ‘__builtin_strncpy’ specified bound 100 equals destination size [-Werror=stringop-truncation] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > ... but we apparently really want to do a strncpy here - the size is already > checked with the assert() statement right in front of it. To silence the > warning, simply replace it with our strpadcpy() function. > > Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com> (two years ago) > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > migration/global_state.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/migration/global_state.c b/migration/global_state.c > index 25311479a4..a33947ca32 100644 > --- a/migration/global_state.c > +++ b/migration/global_state.c > @@ -44,8 +44,8 @@ void global_state_store_running(void) > { > const char *state = RunState_str(RUN_STATE_RUNNING); > assert(strlen(state) < sizeof(global_state.runstate)); > - strncpy((char *)global_state.runstate, > - state, sizeof(global_state.runstate)); > + strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate), > + state, '\0'); > } > > bool global_state_received(void) > -- > 2.18.2 > > Hi Thomas, FIY, I couldn't reproduce the complaint from GCC. I've tested it on focal, "gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0", with QEMU 5df6c87e8. After a succesfull "configure --target-list=x86_64-softmmu && meson compile" build, I tried to manually enable meson's "werror" option, and found no difference. Then, I manually ran gcc, with a couple of "-Werror" variations, such as: cc -Ilibcommon.fa.p -I. -I../../src/qemu -Iqapi -Itrace -Iui \ -Iui/shader -I/usr/include/libpng16 -I/usr/include/libmount \ -I/usr/include/blkid -I/usr/include/glib-2.0 \ -I/usr/lib/x86_64-linux-gnu/glib-2.0/include \ -I/usr/include/gio-unix-2.0 -I/root/src/qemu/slirp/src -Islirp/src \ -I/usr/include/pixman-1 -I/usr/include/gtk-3.0 \ -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 \ -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include \ -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/fribidi \ -I/usr/include/harfbuzz -I/usr/include/atk-1.0 -I/usr/include/uuid \ -I/usr/include/freetype2 -I/usr/include/gdk-pixbuf-2.0 -Ilinux-headers \ -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror \ -Werror=stringop-truncation -std=gnu99 -O2 -g -U_FORTIFY_SOURCE \ -D_FORTIFY_SOURCE=2 -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 \ -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef \ -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common \ -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits \ -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers \ -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined \ -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi \ -fstack-protector-strong -iquote /root/src/qemu/tcg/i386 -isystem \ /root/src/qemu/linux-headers -iquote . -iquote /root/src/qemu -iquote \ /root/src/qemu/accel/tcg -iquote /root/src/qemu/include -iquote \ /root/src/qemu/disas/libvixl -pthread -fPIC -MD -MQ \ libcommon.fa.p/migration_global_state.c.o -MF \ libcommon.fa.p/migration_global_state.c.o.d -o \ libcommon.fa.p/migration_global_state.c.o -c \ ../../src/qemu/migration/global_state.c But I could not trigger the warning (and thus error). The change here looks good, but I thought I should let you know, and maybe I'm missing something obvious. Thanks, - Cleber.
On 21/09/2020 22.39, Cleber Rosa wrote: > On Fri, Sep 18, 2020 at 12:34:27PM +0200, Thomas Huth wrote: >> GCC 9.3.0 on Ubuntu complains: >> >> In file included from /usr/include/string.h:495, >> from /home/travis/build/huth/qemu/include/qemu/osdep.h:87, >> from ../migration/global_state.c:13: >> In function ‘strncpy’, >> inlined from ‘global_state_store_running’ at ../migration/global_state.c:47:5: >> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: >> ‘__builtin_strncpy’ specified bound 100 equals destination size [-Werror=stringop-truncation] >> 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> ... but we apparently really want to do a strncpy here - the size is already >> checked with the assert() statement right in front of it. To silence the >> warning, simply replace it with our strpadcpy() function. >> >> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com> (two years ago) >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> migration/global_state.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/migration/global_state.c b/migration/global_state.c >> index 25311479a4..a33947ca32 100644 >> --- a/migration/global_state.c >> +++ b/migration/global_state.c >> @@ -44,8 +44,8 @@ void global_state_store_running(void) >> { >> const char *state = RunState_str(RUN_STATE_RUNNING); >> assert(strlen(state) < sizeof(global_state.runstate)); >> - strncpy((char *)global_state.runstate, >> - state, sizeof(global_state.runstate)); >> + strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate), >> + state, '\0'); >> } >> >> bool global_state_received(void) >> -- >> 2.18.2 >> >> > > Hi Thomas, > > FIY, I couldn't reproduce the complaint from GCC. I've tested it on focal, > "gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0", with QEMU 5df6c87e8. Hi Cleber, I've hit the error here: https://travis-ci.com/github/huth/qemu/jobs/385871010#L2930 It seems to use the very same compiler version as you did, so that's kind of weird... Maybe it's related to the other compiler flags, either --enable-gprof, --enable-gcov or --disable-pie ? Thomas
diff --git a/migration/global_state.c b/migration/global_state.c index 25311479a4..a33947ca32 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -44,8 +44,8 @@ void global_state_store_running(void) { const char *state = RunState_str(RUN_STATE_RUNNING); assert(strlen(state) < sizeof(global_state.runstate)); - strncpy((char *)global_state.runstate, - state, sizeof(global_state.runstate)); + strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate), + state, '\0'); } bool global_state_received(void)
GCC 9.3.0 on Ubuntu complains: In file included from /usr/include/string.h:495, from /home/travis/build/huth/qemu/include/qemu/osdep.h:87, from ../migration/global_state.c:13: In function ‘strncpy’, inlined from ‘global_state_store_running’ at ../migration/global_state.c:47:5: /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 100 equals destination size [-Werror=stringop-truncation] 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... but we apparently really want to do a strncpy here - the size is already checked with the assert() statement right in front of it. To silence the warning, simply replace it with our strpadcpy() function. Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com> (two years ago) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- migration/global_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)