linux-kernel - Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Oct 2016 17:44:36 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Will Deacon <will.deacon@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Stephen Boyd <sboyd@...eaurora.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        james.greenhalgh@....com,
        Gregory CLEMENT <gregory.clement@...e-electrons.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness

On Wednesday, October 19, 2016 4:27:51 PM CEST Ard Biesheuvel wrote:
> >
> > Why not turn it into a runtime warning in this driver?
> >
> > diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> > index cecb0fdfaef6..711d1d9842cc 100644
> > --- a/drivers/clk/mvebu/armada-37xx-periph.c
> > +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> > @@ -349,8 +349,10 @@ static int armada_3700_add_composite_clk(const struct clk_periph_data *data,
> >                         rate->reg = reg + (u64)rate->reg;
> >                         for (clkt = rate->table; clkt->div; clkt++)
> >                                 table_size++;
> > -                       rate->width = order_base_2(table_size);
> > -                       rate->lock = lock;
> > +                       if (!WARN_ON(table_size == 0)) {
> > +                               rate->width = order_base_2(table_size);
> > +                               rate->lock = lock;
> > +                       }
> >                 }
> >         }
> >
> 
> I guess Will is not looking for a way to fix the driver, but for a way
> to eliminate this issue entirely going forward.
>
> In general, I think the issue where constant folding results in
> ilog2() or other similar functions being called with invalid build
> time constant parameter values is simply something we have to deal
> with.
> 
> In this case, it is in fact order_base_2() that deviates from its
> documented behavior (as Will points out), and fixing /that/ should
> make this particular issue go away afaict.

Ah, right. I also noticed that order_base_2() is defined as
log2(1 << (log2(n-1)+1)), which seems a bit redundant.
Maybe we can simplify it to something like

#define order_base_2(n) ((n) <= 1) ? 0 : log2((n) - 1) + 1)

	Arnd