On Thu, Sep 11, 2014 at 07:47:51PM +0200, Marek Polacek wrote:
> So, how does this look now?
Looks much better.
There are some nits I'd change, like:
1) no need not to handle bitfields
2) IMHO it should handle PARM_DECL and RESULT_DECL alongside of VAR_DECL
3) decl_p IMHO should use just DECL_P
4) it doesn't make sense to build ADDR_EXPR if you are never going to use
it
Attaching some (incomplete) testcase I've used to look at the
implementation, e.g. f1 is for testing how it plays together with
-fsanitize=bounds (perhaps, maybe as a follow-up, we could optimize by
looking for UBSAN_BOUNDS call with expected arguments in a few statements
before array access (e.g. remember last UBSAN_BOUNDS seen in the same
basic block) if inner is DECL_P), f2 to show a case which -fsanitize=bounds
doesn't instrument, but -fsanitize=object-size should, f3/f4 the same with
PARM_DECL, f5/f6 unsuccessful attempt for RESULT_DECL, f7/f8 bitfield
tests, f9 something where __bos is folded very early (already before objsz1
pass), f10 where __bos is folded during objsz1 pass.
If you want to turn parts of that testcase into real /ubsan/ tests, go
ahead. Other than that, if you are fine with following changes, can you
incorporate them into the patch and retest?
Thanks.
--- gcc/ubsan.c.jj 2014-10-02 12:26:30.000000000 +0200
+++ gcc/ubsan.c 2014-10-02 12:57:40.131267225 +0200
@@ -1421,11 +1421,21 @@ instrument_object_size (gimple_stmt_iter
switch (TREE_CODE (t))
{
- case ARRAY_REF:
case COMPONENT_REF:
+ if (TREE_CODE (t) == COMPONENT_REF
+ && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)) != NULL_TREE)
+ {
+ tree repr = DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1));
+ t = build3 (COMPONENT_REF, TREE_TYPE (repr), TREE_OPERAND (t, 0),
+ repr, NULL_TREE);
+ }
+ break;
+ case ARRAY_REF:
case INDIRECT_REF:
case MEM_REF:
case VAR_DECL:
+ case PARM_DECL:
+ case RESULT_DECL:
break;
default:
return;
@@ -1446,8 +1456,7 @@ instrument_object_size (gimple_stmt_iter
|| bitsize != size_in_bytes * BITS_PER_UNIT)
return;
- bool decl_p = VAR_P (inner) || TREE_CODE (inner) == PARM_DECL
- || TREE_CODE (inner) == RESULT_DECL;
+ bool decl_p = DECL_P (inner);
tree base = decl_p ? inner : TREE_OPERAND (inner, 0);
tree ptr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (t)), t);
@@ -1464,14 +1473,15 @@ instrument_object_size (gimple_stmt_iter
break;
}
- if (!POINTER_TYPE_P (TREE_TYPE (base)) && !VAR_P (base))
+ if (!POINTER_TYPE_P (TREE_TYPE (base)) && !DECL_P (base))
return;
tree sizet;
- tree base_addr = build1 (ADDR_EXPR,
- build_pointer_type (TREE_TYPE (base)), base);
- unsigned HOST_WIDE_INT size = compute_builtin_object_size (decl_p ? base_addr
- : base, 0);
+ tree base_addr = base;
+ if (decl_p)
+ base_addr = build1 (ADDR_EXPR,
+ build_pointer_type (TREE_TYPE (base)), base);
+ unsigned HOST_WIDE_INT size = compute_builtin_object_size (base_addr, 0);
if (size != (unsigned HOST_WIDE_INT) -1)
sizet = build_int_cst (sizetype, size);
else if (optimize)
@@ -1479,7 +1489,7 @@ instrument_object_size (gimple_stmt_iter
/* Generate __builtin_object_size call. */
sizet = builtin_decl_explicit (BUILT_IN_OBJECT_SIZE);
- sizet = build_call_expr_loc (loc, sizet, 2, decl_p ? base_addr : base,
+ sizet = build_call_expr_loc (loc, sizet, 2, base_addr,
integer_zero_node);
sizet = force_gimple_operand_gsi (gsi, sizet, false, NULL_TREE, true,
GSI_SAME_STMT);
@@ -1492,8 +1502,7 @@ instrument_object_size (gimple_stmt_iter
/* ptr + sizeof (*ptr) - base */
t = fold_build2 (MINUS_EXPR, sizetype,
fold_convert (pointer_sized_int_node, ptr),
- fold_convert (pointer_sized_int_node,
- decl_p ? base_addr : base));
+ fold_convert (pointer_sized_int_node, base_addr));
t = fold_build2 (PLUS_EXPR, sizetype, t, TYPE_SIZE_UNIT (type));
/* Perhaps we can omit the check. */
Jakub
struct T { char d[8]; int e; };
struct T t = { "abcdefg", 1 };
#ifdef __cplusplus
struct C { C () : d("abcdefg"), e(1) {} C (const C &x) { __builtin_memcpy (d,
x.d, 8); e = x.e; } ~C () {} char d[8]; int e; };
#endif
struct U { int a : 5; int b : 19; int c : 8; };
struct S { struct U d[10]; };
struct S s;
int
f1 (int i)
{
return t.d[i];
}
int
f2 (int i)
{
char *p = t.d;
p += i;
return *p;
}
int
f3 (struct T x, int i)
{
return x.d[i];
}
int
f4 (struct T x, int i)
{
char *p = x.d;
p += i;
return *p;
}
#ifdef __cplusplus
struct C
f5 (int i)
{
struct C x;
x.d[i] = 'z';
return x;
}
struct C
f6 (int i)
{
struct C x;
char *p = x.d;
p += i;
*p = 'z';
return x;
}
#endif
int
f7 (int i)
{
return s.d[i].b;
}
int
f8 (int i)
{
struct U *u = s.d;
u += i;
return u->b;
}
int
f9 (void)
{
char *p = t.d;
p += 4;
return *p;
}
int
f10 (int x)
{
char *p = t.d + (x ? 6 : 3);
return *p;
}
