On Thu, Nov 06, 2014 at 11:19:08PM +0100, Marek Polacek wrote:
> First part of this patch is about removing the useless check that
> we talked about earlier today.
>
> The rest is about not emitting UBSAN_OBJECT_SIZE checks (those often
> come with multiple statements to compute a pointer difference) for
> ARRAY_REFs that are already instrumented by UBSAN_BOUNDS.
>
> I do this by moving the UBSAN_OBJECT_SIZE instrumentation so that
> it is done first in the ubsan pass - then I can just check whether
> the statement before that ARRAY_REF is a UBSAN_BOUND check. If it
> is, it should be clear that it is checking the ARRAY_REF, and I can
> drop the UBSAN_OBJECT_SIZE check. (Moving the UBSAN_OBJECT_SIZE
> instrumentation means that there won't be e.g. UBSAN_NULL check in
> between the ARRAY_REF and UBSAN_BOUND.)
>
> Earlier, I thought I should check that both UBSAN_OBJECT_SIZE and
> UBSAN_BOUND checks are checking the same array index, but that
> wouldn't work for multidimensional arrays, and just should not be
> needed.
IMHO it is needed and is highly desirable, otherwise you risk missed
diagnostics from -fsanitize=object-size when it is needed.
Consider e.g.:
extern int a[][10][10];
int
foo (int x, int y, int z)
{
return a[x][y][z];
}
int a[10][10][10] = {};
testcase, here only the y and z indexes are bounds checked, but
the x index is not (UBSAN_BOUNDS is added early, before the a var
definition is parsed, while ubsan pass runs afterwards, so can know the
object size.
If you have a multi-dimensional array, you can just walk backwards
within the same bb, looking for UBSAN_BOUNDS calls that verify
the indexes where needed.
Say on:
struct S { int a:3; };
extern struct S a[][10][10];
int
foo (int x, int y, int z)
{
return a[5][11][z].a;
}
struct S a[10][10][10] = {};
you have:
UBSAN_BOUNDS (0B, 11, 9);
z.0_4 = z_3(D);
UBSAN_BOUNDS (0B, z.0_4, 9);
_6 = a[5][11][z.0_4].a;
and you walk the handled components:
1) COMPONENT_REF - ok
2) ARRAY_REF with index z.0_4 and array index maximum is 9, there is
UBSAN_BOUNDS right above it checking that
3) ARRAY_REF with index 11; 11 is bigger than index maximum 9,
there is UBSAN_BOUNDS call for it in the same bb
4) ARRAY_REF with index 5; 5 is smaller or equal than index maximum 9,
no UBSAN_BOUNDS is needed
5) decl inside of the innermost handled component, we can avoid
the object-size instrumentation; if the base is not a decl,
never omit object-size instrumentation.
Jakub
