Re: [Xen-devel] Ping: [PATCH 2/2] x86/mtrr: fix build with gcc9

["Jan Beulich" <JBeulich@suse.com>]

>>> On 17.06.19 at 17:47, <andrew.cooper3@citrix.com> wrote:
> On 14/06/2019 16:56, Jan Beulich wrote:
>>>>> On 07.03.19 at 11:32,  wrote:
>>> generic.c: In function ‘print_mtrr_state’:
>>> generic.c:210:11: error: ‘%0*lx’ directive output between 1 and 1073741823 
>>> bytes may cause result to exceed ‘INT_MAX’ [-Werror=format-overflow=]
>>>   210 |    printk("%s  %u base %0*"PRIx64"000 mask %0*"PRIx64"000 %s\n",
>>>       |           ^~~~~~~~~~~~~~~~~
>>> generic.c:210:44: note: format string is defined here
>>>   210 |    printk("%s  %u base %0*"PRIx64"000 mask %0*"PRIx64"000 %s\n",
>>> generic.c:210:11: note: directive argument in the range [0, 
>>> 4503599627370495]
>>>   210 |    printk("%s  %u base %0*"PRIx64"000 mask %0*"PRIx64"000 %s\n",
>>>       |           ^~~~~~~~~~~~~~~~~
>>> generic.c:210:11: note: assuming directive output of 1 byte
>>>
>>> Restrict the width of the variable "width" controlling the number of
>>> address digits output.
>>>
>>> Reported-by: Charles Arnold <carnold@suse.com>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> This one's still pending for us to build cleanly with gcc 9.
> 
> I can't reproduce it with any build of GCC 9 (all of which are straight
> from the upstream tree).  Is this a locally patched version?

This was with a pre-RC version of gcc9; I wouldn't be surprised if they
changed their code before the release. I admit it didn't occur to me to
retry building without the patch.

>> I know you'd like it be done differently, but I'm not happy with the
>> implications of your suggestion, and I've explained why.
> 
> But you haven't adequately (IMO) addressed any of the shortcomings. 
> Notably that the argument falls down on all common Intel platforms, and
> its still a piece of magic which only you know how to interpret.

I'm unaware of shortcomings, and the "magics" of the number of digits
logged is simply not relevant to people now knowing of this. It is
relevant to people like me who do know. And I can't do anything about
the number of address bits on common Intel platforms not being evenly
divisible by 4.

>> I would
>> (hesitantly, i.e. just to get the build issue out of the way) ack
>> your variant if you submitted it, but I'd appreciate if you would
>> re-consider whether you could live with going with the one here.
> 
> I'm happy to put a SoB and real commit message on my patch, but I'd like
> to actually get to the bottom of the build failure, given that it hasn't
> been reproduced by anyone else using GCC 9.
> 
> I don't view limiting the type as a viable fix, because all does is try
> to game whichever piece of logic GCC is using to object to the
> construct, and is therefore likely to break again in the future.

Well, I can't exclude this happening, but the mention of INT_MAX in
the diagnostic doesn't seem to make this very likely with an 8-bit wide
width specifier.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel