Re: [PATCH 2/2] x86/mtrr: fix build with gcc9

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Charles Arnold <CARNOLD@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Wei Liu <wei.liu2@citrix.com>, Juergen Gross <jgross@suse.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 2/2] x86/mtrr: fix build with gcc9
Date: Thu, 7 Mar 2019 15:20:18 +0100	[thread overview]
Message-ID: <20190307142018.znh74smoi4nnt4ds@Air-de-Roger> (raw)
In-Reply-To: <5C80FEE5020000780021C6D5@prv1-mh.provo.novell.com>

On Thu, Mar 07, 2019 at 04:22:13AM -0700, Jan Beulich wrote:
> >>> On 07.03.19 at 11:55, <roger.pau@citrix.com> wrote:
> > On Thu, Mar 07, 2019 at 03:32:12AM -0700, Jan Beulich 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.
> > 
> > I have to admit I'm not sure why gcc complains here, and why switching
> > to unsigned char fixes it. unsigned char max value would be 255, which
> > when used as a width to print an unsigned long it's also too high?
> 
> With
> 
> 	width = (paddr_bits - PAGE_SHIFT + 3) / 4;
> 
> gcc simply can't know that the valid value range is rather small. Hence
> it assumes millions of leading zeros _could_ be printed. Their doc
> explicitly suggests using a more narrow type, which is why ...
> 
> > Does checking that width <= 16 also placate gcc?
> 
> ... I didn't even try this option (despite having considered it).

Ack, if that's the recommended way. I would prefer using unsigned int
and adding a BUG_ON if > 16. Anyway:

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

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