Re: [Qemu-block] [Qemu-devel] Fwd: [BUG] Failed to compile using gcc7.1

[Max Reitz]

On 2017-06-12 05:19, Philippe Mathieu-Daudé wrote:
> Hi Tsung-en,
> 
> On 06/11/2017 04:08 PM, Tsung-en Hsiao wrote:
>> Hi all,
>> I encountered the same problem on gcc 7.1.1 and found Qu's mail in
>> this list from google search.
>>
>> Temporarily fix it by specifying the string length in snprintf
>> directive. Hope this is helpful to other people encountered the same
>> problem.
> 
> Thank your for sharing this.
> 
>>
>> @@ -1,9 +1,7 @@
>> ---
>> --- a/block/blkdebug.c
>> -                 "blkdebug:%s:%s", s->config_file ?: "",
>> --- a/block/blkverify.c
>> -                 "blkverify:%s:%s",
>> --- a/hw/usb/bus.c
>> -        snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
>> -        snprintf(downstream->path, sizeof(downstream->path), "%d",
>> portnr);
>> -- 
>> +++ b/block/blkdebug.c
>> +                 "blkdebug:%.2037s:%.2037s", s->config_file ?: "",
> 
> It is a rather funny way to silent this warning :) Truncating the
> filename until it fits.
> 
> However I don't think it is the correct way since there is indeed an
> overflow of bs->exact_filename.
> 
> Apparently exact_filename from "block/block_int.h" is defined to hold a
> pathname:
>     char exact_filename[PATH_MAX];
> 
> but is used for more than that (for example in blkdebug.c it might use
> until 10+2*PATH_MAX chars).

In any case, truncating the filenames will do just as much as truncating
the result: You'll get an unusable filename.

> I suppose it started as a buffer to hold a pathname then more block
> drivers were added and this buffer ended used differently.
> 
> If it is a multi-purpose buffer one safer option might be to declare it
> as a GString* and use g_string_printf().

What it is supposed to be now is just an information string we can print
to the user, because strings are nicer than JSON objects. There are some
commands that take a filename for identifying a block node, but I dream
we can get rid of them in 3.0...

The right solution is to remove it altogether and have a
"char *bdrv_filename(BlockDriverState *bs)" function (which generates
the filename every time it's called). I've been working on this for some
years now, actually, but it was never pressing enough to get it finished
(so I never had enough time).

What we can do in the meantime is to not generate a plain filename if it
won't fit into bs->exact_filename.

(The easiest way to do this probably would be to truncate
bs->exact_filename back to an empty string if snprintf() returns a value
greater than or equal to the length of bs->exact_filename.)

What to do about hw/usb/bus.c I don't know (I guess the best solution
would be to ignore the warning, but I don't suppose that is going to work).

Max

> 
> I CC'ed the block folks to have their feedback.
> 
> Regards,
> 
> Phil.
> 
>> +++ b/block/blkverify.c
>> +                 "blkverify:%.2038s:%.2038s",
>> +++ b/hw/usb/bus.c
>> +        snprintf(downstream->path, sizeof(downstream->path), "%.12s.%d",
>> +        snprintf(downstream->path, sizeof(downstream->path), "%.12d",
>> portnr);
>>
>> Tsung-en Hsiao
>>
>>> Qu Wenruo Wrote:
>>>
>>> Hi all,
>>>
>>> After upgrading gcc from 6.3.1 to 7.1.1, qemu can't be compiled with
>>> gcc.
>>>
>>> The error is:
>>>
>>> ------
>>>  CC      block/blkdebug.o
>>> block/blkdebug.c: In function 'blkdebug_refresh_filename':
>>>
>>> block/blkdebug.c:693:31: error: '%s' directive output may be
>>> truncated writing up to 4095 bytes into a region of size 4086
>>> [-Werror=format-truncation=]
>>>
>>>                  "blkdebug:%s:%s", s->config_file ?: "",
>>>                               ^~
>>> In file included from /usr/include/stdio.h:939:0,
>>>                 from /home/adam/qemu/include/qemu/osdep.h:68,
>>>                 from block/blkdebug.c:25:
>>>
>>> /usr/include/bits/stdio2.h:64:10: note: '__builtin___snprintf_chk'
>>> output 11 or more bytes (assuming 4106) into a destination of size 4096
>>>
>>>   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>>>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>        __bos (__s), __fmt, __va_arg_pack ());
>>>        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>> make: *** [/home/adam/qemu/rules.mak:69: block/blkdebug.o] Error 1
>>> ------
>>>
>>> It seems that gcc 7 is introducing more restrict check for printf.
>>>
>>> If using clang, although there are some extra warning, it can at
>>> least pass the compile.
>>>
>>> Thanks,
>>> Qu
>>

signature.asc
Description: OpenPGP digital signature