This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 5 Dec 2013 12:37:27 +0100
- Subject: Re: [PATCH] Fix up passing long long in ubsan with -m32 (PR sanitizer/59333)
- Authentication-results: sourceware.org; auth=none
- References: <20131205112625 dot GB11710 at redhat dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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