We've been having "issues" in our branch when exporting to the global
space ranges that take into account previously known ranges
(SSA_NAME_RANGE_INFO, etc). For the longest time we had the export
feature turned off because it had the potential of removing
__builtin_unreachable code early in the pipeline. This was causing one
or two tests to fail.
I finally got fed up, and investigated why.
Take the following code:
i_4 = somerandom ();
if (i_4 < 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
__builtin_unreachable ();
<bb 4> :
It turns out that both legacy evrp and VRP have code that notices the
above pattern and sets the *global* range for i_4 to [0,MAX]. That is,
the range for i_4 is set, not at BB4, but at the definition site. See
uses of assert_unreachable_fallthru_edge_p() for details.
This global range causes subsequent passes (VRP1 in the testcase below),
to remove the checks and the __builtin_unreachable code altogether.
// pr80776-1.c
int somerandom (void);
void
Foo (void)
{
int i = somerandom ();
if (! (0 <= i))
__builtin_unreachable ();
if (! (0 <= i && i <= 999999))
__builtin_unreachable ();
sprintf (number, "%d", i);
}
This means that by the time the -Wformat-overflow warning runs, the
above sprintf has been left unguarded, and a bogus warning is issued.
Currently the above test does not warn, but that's because of an
oversight in export_global_ranges(). This function is disregarding
known global ranges (SSA_NAME_RANGE_INFO and SSA_NAME_PTR_INFO) and only
setting ranges the ranger knows about.
For the above test the IL is:
<bb 2> :
i_4 = somerandom ();
if (i_4 < 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
__builtin_unreachable ();
<bb 4> :
i.0_1 = (unsigned int) i_4;
if (i.0_1 > 999999)
goto <bb 5>; [INV]
else
goto <bb 6>; [INV]
<bb 5> :
__builtin_unreachable ();
<bb 6> :
_7 = __builtin___sprintf_chk (&number, 1, 7, "%d", i_4);
Legacy evrp has determined that the range for i_4 is [0,MAX] per my
analysis above, but ranger has no known range for i_4 at the definition
site. So at export_global_ranges time, ranger leaves the [0,MAX] alone.
OTOH, evrp sets the global range at the definition for i.0_1 to
[0,999999] per the same unreachable feature. However, ranger has
correctly determined that the range for i.0_1 at the definition is
[0,MAX], which it then proceeds to export. Since the current
export_global_ranges (mistakenly) does not take into account previous
global ranges, the ranges in the global tables end up like this:
i_4: [0, MAX]
i.0_1: [0, MAX]
This causes the first unreachable block to be removed in VRP1, but the
second one to remain. Later VRP can determine that i_4 in the sprintf
call is [0,999999], and no warning is issued.
But... the missing bogus warning is due to current export_global_ranges
ignoring SSA_NAME_RANGE_INFO and friends, something which I'd like to
fix. However, fixing this, gets us back to:
i_4: [0, MAX]
i.0_1: [0, 999999]
Which means, we'll be back to removing the unreachable blocks and
issuing a warning in pr80776-1.c (like we have been since the beginning
of time).
The attached patch fixes export_global_ranges to the expected behavior,
and adds the previous XFAIL to pr80776-1.c, while documenting why this
warning is issued in the first place.
Once legacy evrp is removed, this won't be an issue, as ranges in the IL
will tell the truth. However, this will mean that we will no longer
remove the first __builtin_unreachable combo. But ISTM, that would be
correct behavior ??.
BTW, in addition to this patch we could explore removing the
assert_unreachable_fallthru_edge_p() use in the evrp_analyzer, since it
is no longer needed to get the warnings in the testcases in the original
PR correctly (gcc.dg/pr80776-[12].c).
