On Thu, Dec 05, 2013 at 12:37:27PM +0100, Jakub Jelinek wrote:
> On Thu, Dec 05, 2013 at 12:26:25PM +0100, Marek Polacek wrote:
> > --- gcc/ubsan.h.mp 2013-12-05 11:25:18.979469651 +0100
> > +++ gcc/ubsan.h 2013-12-05 11:25:28.958507098 +0100
> > @@ -41,7 +41,7 @@ extern tree ubsan_instrument_unreachable
> > extern tree ubsan_create_data (const char *, location_t,
> > const struct ubsan_mismatch_data *, ...);
> > extern tree ubsan_type_descriptor (tree, bool);
> > -extern tree ubsan_encode_value (tree);
> > +extern tree ubsan_encode_value (tree, bool);
>
> I'd do extern tree ubsan_encode_value (tree, bool = false);
> so that we at least have some advantage from the C->C++ switch
> occassionally. Then you can keep most callers of ubsan_encode_value
> as is.
Cool, done.
> > + {
> > + tree itype = build_nonstandard_integer_type (bitsize, true);
> > + t = fold_build1 (VIEW_CONVERT_EXPR, itype, t);
> > + return fold_convert (pointer_sized_int_node, t);
> > + }
> > + default:
> > + gcc_unreachable ();
> > + }
> > + else
> > + {
> > + if (!TREE_ADDRESSABLE (t))
> > + {
> > + /* The reason for this is that we don't want to pessimize
> > + code by making vars unnecessarily addressable. */
> > + tree var = create_tmp_var (type, NULL);
> > + tree tem = build2 (MODIFY_EXPR, void_type_node, var, t);
> > + if (in_expand_p)
> > + {
> > + SET_DECL_RTL (var,
> > + assign_stack_temp_for_type (TYPE_MODE (type),
> > + GET_MODE_SIZE (TYPE_MODE (type)), type));
>
> Formatting looks wrong, I'd do:
> rtx mem
> = assign_stack_temp_for_type (TYPE_MODE (type),
> GET_MODE_SIZE (TYPE_MODE (type)),
> type);
> SET_DECL_RTL (var, mem);
Ok.
> > + return build_fold_addr_expr (var);
>
> This is something I don't understand. What will assign the value to var
> aka mem then?
Bah, fixed.
> > + }
> > + t = build_fold_addr_expr (var);
> > + return build2 (COMPOUND_EXPR, TREE_TYPE (t), tem, t);
>
> I would expect you want to use this too even if in_expand_p...
Unfortunately, this won't be sufficient -- in expand_expr_real_1 we
have
case COMPOUND_EXPR:
/* Lowered by gimplify.c. */
gcc_unreachable ();
and with the code above we hit that.
> > --- gcc/testsuite/c-c++-common/ubsan/pr59333.c.mp 2013-12-05
> > 11:30:36.984759390 +0100
> > +++ gcc/testsuite/c-c++-common/ubsan/pr59333.c 2013-12-05
> > 11:31:36.599979040 +0100
> > @@ -0,0 +1,8 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-fsanitize=undefined" } */
> > +
> > +long long int
> > +foo (long long int i, long long int j)
> > +{
> > + return i * j;
> > +}
>
> Please add a runtime testcase that verifies the values
> instead. As it is long long int, you are guaranteed
> it is at least 64-bit, so just make the function __attribute__((noinline,
> noclone)), pass some well chosen small constants so that it overflows
> and dg-output whether the library prints the correct values.
Will do.
Marek
