On 11/24/2016 05:11 AM, Richard Biener wrote:
where CST is unsigned implying that the lower bound of the offset
is the greater of CST and MIN. For instance, in the following it
determines that bos(p, 0) is 4 (and if the 3 were greater than 7
and overflowed the addition the result would be zero). I'm not
sure I understand what you suggest I do differently to make this
work.
char d[7];
#define bos(p, t) __builtin_object_size (p, t)
long f (unsigned i)
{
if (2 < i) i = 2;
char *p = &d[i] + 3;
return bos (p, 0);
}
I'm sure that doesn't work as you match for PLUS_EXPR.
+ else if (range_type == VR_ANTI_RANGE)
+ {
+ offrange[0] = max.to_uhwi () + 1;
+ offrange[1] = min.to_uhwi () - 1;
+ return true;
+ }
first of all, how do you know it fits uhwi? Second, from ~[5, 9] you get
[10, 4] !? That looks bogus (and contrary to the function comment
postcondition)
I admit I have some trouble working with anti-ranges. It's also
difficult to fully exercise them in this pass because it only runs
after EVRP but not after VRP1 (except with -g), so only limited
range information is available. (I'm hoping to eventually change
it but moving the passes broke a test in a way that seemed too
complex to fix for this project).
Maybe simply ignore VR_ANTI_RANGEs for now then?
That's where I'd start. I can see how handling an anti-range might be
useful, but I don't think it's as important as the rest of the stuff
we're trying to catch here.
So I'd echo Richi's suggestion, punt VR_ANTI_RANGE for now.
The code above is based on the observation that an anti-range is
often used to represent the full subrange of a narrower signed type
like signed char (as ~[128, -129]). I haven't been able to create
an anti-range like ~[5, 9]. When/how would a range like that come
about (so I can test it and implement the above correctly)?
if (a < 4)
if (a > 8)
b = a;
then b should have ~[5, 9]
Right.
Note a trick VRP uses internally is to treat an anti-range as
the union of two ranges. ~[X, Y] == [MIN, X-1] u [Y+1, MAX].
That essentially removes anti-range handling from VRPs
propagation - not sure if it would help your case.
It certainly helps Andrew, but he's working on a totally different problem.
+ else if (range_type == VR_VARYING)
+ {
+ gimple *def = SSA_NAME_DEF_STMT (off);
+ if (is_gimple_assign (def))
+ {
+ tree_code code = gimple_assign_rhs_code (def);
+ if (code == NOP_EXPR)
+ {
please trust range-info instead of doing your own little VRP here.
VR_VARYING -> return false.
I would prefer to rely on the range information and not have to work
around it like I do here but, unfortunately, it doesn't always appear
to be available. For example, in the following test case:
I find myself in agreement with Richi on this.
I *would* suggest taking those cases where you are enhancing the results
you get from VRP and turning those into xfailed's testcases. Those
tests represent future work :-)
get_range_info doesn't accept pointers at all (perhaps that's what
you meant by "VRP on pointers is limited"). In Gimple, &a[i] + 5
is represented as just that (i.e., _1 = &a[i_3]; p_6 = _1 + 5;) and
so to get the right answer for bos(&a[i] + 5) the patch jumps though
some hoops. But again, I could very well be missing something that's
obvious to you so if you think that's the case I'd be happy to simplify
the code if you point me in the right direction.
Yes, get_range_info is limited. IMHO of you want to enhance object-size
to cover ranges by implicitely working on a different IL then it probably
should be re-written with that in mind. The EVRP pass provides a
good template for how to re-use the VRP machinery and decomposing
the lattice a bit further into BASE + range shouldn't be difficult.
I'm leaving it to others if we have to have this improvement for GCC 7
(bos has its own share of know issues besides missing features).
It falls into "it'd be nice, but it's not critical". Incrementally
improving b_o_s is seen as a high value target because doing so improves
our ability to statically detect buffer overflows.
The question is can we improve b_o_s without making the pass even more
incomprehensible.
Seems like Martin has a follow-up patch, so I'll wait to see that.
jeff
