On Fri, Sep 16, 2016 at 10:44:34AM -0600, Jeff Law wrote: > On 08/21/2016 02:30 PM, Eric Botcazou wrote: > > > Consider the case where sym1 results in a non-null return value (and > > > initializes neg1/inv1), but sym2 results in a null return value, leaving > > > neg2/inv2 undefined, but cst2 is can still be true (ADDR_EXPR with an > > > invariant address comes to mind). > > > > > > Thus we can get into these statements: > > > > > > > > > tree cst = cst1 ? val1 : val2; > > > tree inv = cst1 ? inv2 : inv1; > > > > > > > > > Note carefully how they test cst1 and depending on its value, they may > > > read val2 or inv2. > > > > The key here is that cst1 cannot be true if sym1 is non-null, same for cst2 > > and sym2. > > > > The code is guarded with: > > > > /* If one is of the form '[-]NAME + CST' and the other is constant, then > > it might be possible to say something depending on the constants. */ > > if ((sym1 && inv1 && cst2) || (sym2 && inv2 && cst1)) > > > > If this is the first case, then cst1 is false and val2 and inv1 are read. > > If this is the second case, then cst1 is true and val1 and inv2 are read. > > > > So inv2 is read only in the second case, and is initialized. > THanks. Sometimes this stuff gets painful to follow :( > > It looks like Andi already checked in this change. Andi, please don't do > that. THe patch wasn't approved at the time of your commit and there's no > indication the change was regression tested.
I don't post any patches that are not regression tested. In fact the patch was needed to run proper regression tests with profiling on because it was a bootstrap fix. I had interpreted your earlier comments as approval, and imho the patch is obvious anyways. Also I would like to point out that such a long turn around time for bootstrap fixes (and only out of secondary considerations which had nothing to do with the actual bootstrap problem) is extremely disruptive for anyone who actually relies on working bootstraps. If there's no intention to keep --enable-werror working for all build options the default should probably change to --disable-werror. -Andi
