This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)


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.

> +	{
> +	  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);

> +	      return build_fold_addr_expr (var);

This is something I don't understand.  What will assign the value to var
aka mem then?

> +	    }
> +	  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...

> --- 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.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]