[PATCH] PCI: Fix BUILD_BUG_ON usage for old gcc

Alex Williamson posted 1 patch 12 months ago
There is a newer version of this series
drivers/pci/probe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] PCI: Fix BUILD_BUG_ON usage for old gcc
Posted by Alex Williamson 12 months ago
As reported in the below link, it seems older versions of gcc cannot
determine that the howmany variable is known for all callers.  Include
a test so that newer compilers can still enforce this sanity check and
older compilers can still work.

Fixes: 4453f360862e ("PCI: Batch BAR sizing operations")
Link: https://lore.kernel.org/all/20250209154512.GA18688@redhat.com
Reported-by: Oleg Nesterov <oleg@redhat.com>
Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Verified against gcc 14.2.1 to still trigger a build error if called
with a constant value greater than 6, Oleg to confirm build issue is
resolved for gcc 5.3.1.

 drivers/pci/probe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b6536ed599c3..1bde89d0dc0d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
 	unsigned int pos, reg;
 	u16 orig_cmd;
 
-	BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS);
+	BUILD_BUG_ON(__builtin_constant_p(howmany) &&
+		     howmany > PCI_STD_NUM_BARS);
 
 	if (dev->non_compliant_bars)
 		return;
-- 
2.47.1
Re: [PATCH] PCI: Fix BUILD_BUG_ON usage for old gcc
Posted by Oleg Nesterov 12 months ago
On 02/10, Alex Williamson wrote:
>
> As reported in the below link, it seems older versions of gcc cannot
> determine that the howmany variable is known for all callers.  Include
> a test so that newer compilers can still enforce this sanity check and
> older compilers can still work.
>
> Fixes: 4453f360862e ("PCI: Batch BAR sizing operations")
> Link: https://lore.kernel.org/all/20250209154512.GA18688@redhat.com
> Reported-by: Oleg Nesterov <oleg@redhat.com>
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
  ^^^^^^^^^^^^

Well, thanks, but I didn't ;)

> @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
>  	unsigned int pos, reg;
>  	u16 orig_cmd;
>  
> -	BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS);
> +	BUILD_BUG_ON(__builtin_constant_p(howmany) &&
> +		     howmany > PCI_STD_NUM_BARS);

Thanks!

Tested-by: Oleg Nesterov <oleg@redhat.com>
Re: [PATCH] PCI: Fix BUILD_BUG_ON usage for old gcc
Posted by Oleg Nesterov 12 months ago
On 02/10, Oleg Nesterov wrote:
>
> On 02/10, Alex Williamson wrote:
> >
> > @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
> >  	unsigned int pos, reg;
> >  	u16 orig_cmd;
> >
> > -	BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS);
> > +	BUILD_BUG_ON(__builtin_constant_p(howmany) &&
> > +		     howmany > PCI_STD_NUM_BARS);
>
> Thanks!
>
> Tested-by: Oleg Nesterov <oleg@redhat.com>

Just in case... I agree with David, statically_true() looks a bit
better and

	BUILD_BUG_ON(statically_true(howmany > PCI_STD_NUM_BARS));

also works for me, so if you decide to update this patch feel free
to keep my Tested-by.

Oleg.
Re: [PATCH] PCI: Fix BUILD_BUG_ON usage for old gcc
Posted by Mitchell Augustin 12 months ago
On Ubuntu's (modern) GCC 13.3.0-6ubuntu2~24.04, both proposed options
also build as expected for me when howmany is set correctly, and fail
as expected when I force howmany = 7.

Tested-by: Mitchell Augustin <mitchell.augustin@canonical.com>

On Tue, Feb 11, 2025 at 4:05 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 02/10, Oleg Nesterov wrote:
> >
> > On 02/10, Alex Williamson wrote:
> > >
> > > @@ -345,7 +345,8 @@ static void pci_read_bases(struct pci_dev *dev, unsigned int howmany, int rom)
> > >     unsigned int pos, reg;
> > >     u16 orig_cmd;
> > >
> > > -   BUILD_BUG_ON(howmany > PCI_STD_NUM_BARS);
> > > +   BUILD_BUG_ON(__builtin_constant_p(howmany) &&
> > > +                howmany > PCI_STD_NUM_BARS);
> >
> > Thanks!
> >
> > Tested-by: Oleg Nesterov <oleg@redhat.com>
>
> Just in case... I agree with David, statically_true() looks a bit
> better and
>
>         BUILD_BUG_ON(statically_true(howmany > PCI_STD_NUM_BARS));
>
> also works for me, so if you decide to update this patch feel free
> to keep my Tested-by.
>
> Oleg.
>


-- 
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering