On Fri, Sep 16, 2016 at 5:29 AM, Martin Sebor <mse...@gmail.com> wrote:
> __builtin_object_size fails for POINTER_PLUS expressions involving
> non-constant offsets into objects of known size, causing GCC to fail
> to detect (and add instrumentation to prevent) buffer overflow in
> some basic cases such as the following:
>
> void f (unsigned i)
> {
> char d [3];
> memcpy (d + i, "abcdef", 5);
> }
>
> Since the size of the destination object is known, the call to memcpy
> is guaranteed to write past the end of it regardless of the value of
> the offset.
>
> The attached patch enhances __builtin_object_size to handle this case
> by returning the size of the whole object as the maximum and the size
> of the object minus T_MAX for the type of the offset T as the minimum.
>
> The patch also adds handling of ranges even though only very few cases
> benefit from it because the VRP pass runs after the object size pass.
> The one case that does appear to profit is when the value of the offset
> is constrained by its type, as in
>
> char a [1000];
>
> unsigned g (unsigned char i)
> {
> char *p = a + i;
> return __builtin_object_size (p, 2);
> }
>
> Here get_range_info () lets __builtin_object_size determine that
> the minimum number of bytes between (a + i) and (a + sizeof s) is
> sizeof a - UCHAR_MAX.
>
> The patch results in 64 more checking calls in a Binutils build than
> before.
I believe that you can't simply use the min/max of the ranges the way you
do nor can you assume zero for the minimum size as op1 might be
negative (for POINTER_PLUS_EXPR you need to interpret the offset
as signed).
Unless I completely misremember how tree-object-size.c works, of course.
Say,
char a[1000];
unsigned g (signed char i)
{
char *p = &a[10] + i;
return __builtin_object_size (p, 1);
}
range for i will be [-128, 127] but we'd like to return 1000 here I think.
Richard.
> Martin
>
> PS What would be a good way to arrange for the VRP pass to run before
> the object size pass so that the latter can benefit more from range
> information? As an experiment I added another instance of the VRP
> pass before the object size pass in passes.def and that worked, but
> I suspect that running VRP a third time isn't optimal.
