On 8/10/20 5:47 AM, Aldy Hernandez wrote:
On 8/6/20 9:30 PM, Martin Sebor wrote:
On 8/6/20 8:53 AM, Aldy Hernandez via Gcc-patches wrote:
+ // Remove the unknown parts of a multi-range.
+ // This will transform [5,10][20,MAX] into [5,10].
Is this comment correct? Wouldn't this result in returning smaller
sizes than the actual value allows? If so, I'd expect that to cause
false positives (and in that case, if none of our tests fail we need
to add some that would).
By my reading of the code below it seems to return the upper range
(i.e., [20, MAX]) but I'm not fully acquainted with the new ranger
APIs yet.
I believe the comment is correct. What I'm trying to do is remove
[X,TYPE_MAX_VALUE] from the range
+ int pairs = positives.num_pairs ();
+ if (pairs > 1
+ && positives.upper_bound () == wi::to_wide (TYPE_MAX_VALUE
(exptype)))
{
- if (integral)
- {
- /* Use the full range of the type of the expression when
- no value range information is available. */
- range[0] = TYPE_MIN_VALUE (exptype);
- range[1] = TYPE_MAX_VALUE (exptype);
- return true;
- }
-
- range[0] = NULL_TREE;
- range[1] = NULL_TREE;
- return false;
+ value_range last_range (exptype,
+ positives.lower_bound (pairs - 1),
+ positives.upper_bound (pairs - 1), VR_ANTI_RANGE);
+ positives.intersect (last_range);
}
Notice that we start with positives == vr (where vr is the range
returned by determine_value_range).
Then we build last_range which is the *opposite* of [X,TYPE_MAX_VALUE].
Notice the VR_ANTI_RANGE in the constructor.
(Note that the nomenclature VR_ANTI_RANGE is being used in the
constructor to keep with the original API. It means "create the inverse
of this range". Ideally, when the m_kind field disappears along with
VR_ANTI_RANGEs, the enum could become RANGE_INVERSE or something less
confusing.)
Finally, last_range is intersected with positives, which will become
whatever VR had, minus the last sub-range.
Those that make sense, or did I miss something?
If you have a testcase that yields a false positive in my proposed
patch, but not in the original code, please let me know so I can adjust
the patch.
Sorry, I misremembered what get_size_range was used for. It's used
to constrain the maximum size of memory accesses by functions like
memcpy (the third argument). The use case I was thinking of was
to determine the bounds of the size of variably modified objects
(like VLAs). It doesn't look like it's used for that.
With the current use taking the lower bound is the conservative
thing to do and using the upper bound would cause false positives.
With the latter it would be the other way around. So the comment
makes sense to me now.
As an aside, here's a test case I played with. I'd expect it to
trigger a warning because the upper bound of the range of array's
size is less than the lower bound of the range of the number of
bytes being written to it.
void f (void*);
void g (unsigned n1, unsigned n2)
{
if (!(n1 >= 2 && n1 <= 3)) // n1 = [2, 3]
return;
char a[n1];
if (!((n2 >= 5 && n2 <= 10)
|| (n2 >= 20))) // n2 = [5, 10] U [20, UINT_MAX]
return;
__builtin_memset (a, 0, n2);
f (a);
}
But I couldn't get it to either produce a multipart range, or
a union of the two parts (i.e., [5, UINT_MAX]). It makes me
wonder what I'm missing about how such ranges are supposed to
come into existence.
Martin
