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

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Charles Arnold <CARNOLD@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] Ping: [PATCH 2/2] x86/mtrr: fix build with gcc9
Date: Mon, 17 Jun 2019 16:47:41 +0100	[thread overview]
Message-ID: <fb71b7c1-133d-b68d-f4a4-ce788cb7d32a@citrix.com> (raw)
In-Reply-To: <5D03C3B90200007800238676@prv1-mh.provo.novell.com>

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?

> 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 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.

~Andrew

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