[PATCH] Fix a missed case in predicate analysis of the late uninit pass

Richard Biener richard.guenther@gmail.com
Wed Apr 3 11:49:00 GMT 2019


On Mon, Apr 1, 2019 at 5:36 PM Vladislav Ivanishin <vlad@ispras.ru> wrote:
>
> Hi!
>
> This is a fairly trivial change fixing a false negative in
> -Wmaybe-uninitialized.  I am pretty sure this is simply an overlooked
> case (is_value_included_in() is not meant to deal with the case where
> both compare codes are NE_EXPRs, neither does it need to, since their
> handling is trivial).
>
> In a nutshell, what happens is values of v restricted by (v != 2) are
> incorrectly considered a subset of values of v restricted by (v != 1).
> As if "v != 2, therefore v != 1".
>
> This is by no means a gcc-9 regression; I'll ping the patch once stage4
> is over, if needed.
>
> This came up when I was experimenting with moving the uninit passes
> around.  On mainline, the late uninit pass runs very late, so reliably
> triggering the affected path is a bit tricky.  So I created a GIMPLE
> test (it reproduces the behavior precisely, but might be fragile
> w.r.t. future versions of the textual representation) and then with a
> hint from Alexander managed to produce a simple C test.  [By the way,
> the first take was to insert an asm with a lot of newlines to prevent
> the dom pass from rewriting the CFG due to high cost of duplicating
> instructions.  This didn't work out; I think the dom pass does not
> respect sizes of inline asms.  I plan to create a testcase and file a
> bug later.]
>
> I ran regression testing on x86_64-pc-linux-gnu and saw no new
> regressions modulo a handful of flaky UNRESOLVED tests under
> gcc.dg/tree-prof.  `BOOT_CFLAGS="-O -Wno-error=maybe-uninitialized
> -Wmaybe-uninitialized" bootstrap` also succeeded producing no new
> warnings.
>
> OK for stage1?

Hum.  While definitely two NE_EXPR do not work correctly I'd like
to see a positive test since LT_EXPR doesn't work either?  Specifically
the code falls through to test is_value_included_in which seems to
assume code1 == code2.

To me it looks like is_value_includeds comment should be clarified to
say

/* Returns true if all values X satisfying X CMPC VAL satisfy
   X CMPC BOUNDARY.  */

That is, "all values in the range" in the current comment is under-specified
since VAL is just a single value.

So I wonder what testcases regress if we remove the && code2 != NE_EXPR
case?  That way we see what the intention was.  A patch should then
change that to

  if ((code1 != code2)
      || !(<condition on code1> && code2 == NE_EXPR))
   return false;

to explicitely spell out what case was meant here.

Richard.

>
> --
> Vlad



More information about the Gcc-patches mailing list