This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix bootstrap-ubsan
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Jakub Jelinek <jakub at redhat dot com>,Marek Polacek <polacek at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 26 Mar 2014 19:55:34 +0100
- Subject: Re: [PATCH] Fix bootstrap-ubsan
- Authentication-results: sourceware.org; auth=none
- References: <20140326160620 dot GJ1817 at tucnak dot redhat dot com>
On March 26, 2014 5:06:20 PM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>Honza's r208831 change apparently broke bootstrap-ubsan.
>The problem is that it now creates __builtin_unreachable with
>gimple_location where LOCATION_LOCUS is UNKNOWN_LOCATION (comes from
>fnsplit, any ideas what gimple_location to use for the calls if any?)
>and with -fsanitize=undefined we fold that into
>__ubsan_handle_builtin_unreachable call.
>But (already seen several times in the past) instead of emitting
>unknown
>file and 0/0 line/column as location we don't emit anything at all,
>which for unreachable data ICEs and in other cases just crashes in
>libubsan.
>The reason for trying to handle UNKNOWN_LOCATION specially is that some
>data
>structures in libubsan don't have sourceLocation (not the ones we emit
>right
>now though), and guess Marek wanted to be prepared to handle that.
>
>This patch robustifies ubsan_create_data, by turning the location into
>a pointer to location_t, NULL means location should not be present,
>pointer
>to UNKNOWN_LOCATION means we should emit location as <unknown>:0:0,
>pointer to real locations something else.
>
>Bootstrapped/regtested with bootstrap-ubsan and normal, ok for trunk?
Ok.
Thanks,
Richard.
>2014-03-26 Jakub Jelinek <jakub@redhat.com>
>
> * ubsan.h (ubsan_create_data): Change second argument's type
> to const location_t *.
> * ubsan.c (ubsan_source_location): If xloc.file is NULL, set it to
> _("<unknown>").
> (ubsan_create_data): Change second argument to const location_t *PLOC.
> Create Loc field whenever PLOC is non-NULL.
> (ubsan_instrument_unreachable, ubsan_expand_null_ifn,
> ubsan_build_overflow_builtin, instrument_bool_enum_load): Adjust
> callers.
>c-family/
> * c-ubsan.c (ubsan_instrument_division, ubsan_instrument_shift,
> ubsan_instrument_vla, ubsan_instrument_return): Adjust
> ubsan_create_data callers.
>
>--- gcc/ubsan.h.jj 2014-01-03 11:40:57.000000000 +0100
>+++ gcc/ubsan.h 2014-03-26 11:14:32.492987318 +0100
>@@ -38,7 +38,7 @@ struct ubsan_mismatch_data {
>
> extern void ubsan_expand_null_ifn (gimple_stmt_iterator);
> extern tree ubsan_instrument_unreachable (location_t);
>-extern tree ubsan_create_data (const char *, location_t,
>+extern tree ubsan_create_data (const char *, const location_t *,
> const struct ubsan_mismatch_data *, ...);
> extern tree ubsan_type_descriptor (tree, bool);
> extern tree ubsan_encode_value (tree, bool = false);
>--- gcc/ubsan.c.jj 2014-03-26 10:17:41.000000000 +0100
>+++ gcc/ubsan.c 2014-03-26 11:32:57.053941843 +0100
>@@ -46,6 +46,7 @@ along with GCC; see the file COPYING3.
> #include "tree-ssanames.h"
> #include "asan.h"
> #include "gimplify-me.h"
>+#include "intl.h"
>
> /* Map from a tree to a VAR_DECL tree. */
>
>@@ -238,6 +239,8 @@ ubsan_source_location (location_t loc)
> tree type = ubsan_source_location_type ();
>
> xloc = expand_location (loc);
>+ if (xloc.file == NULL)
>+ xloc.file = "<unknown>";
>
> /* Fill in the values from LOC. */
> size_t len = strlen (xloc.file);
>@@ -404,7 +407,7 @@ ubsan_type_descriptor (tree type, bool w
> pointer checking. */
>
> tree
>-ubsan_create_data (const char *name, location_t loc,
>+ubsan_create_data (const char *name, const location_t *ploc,
> const struct ubsan_mismatch_data *mismatch, ...)
> {
> va_list args;
>@@ -412,17 +415,18 @@ ubsan_create_data (const char *name, loc
> tree fields[5];
> vec<tree, va_gc> *saved_args = NULL;
> size_t i = 0;
>+ location_t loc = UNKNOWN_LOCATION;
>
> /* Firstly, create a pointer to type descriptor type. */
> tree td_type = ubsan_type_descriptor_type ();
> TYPE_READONLY (td_type) = 1;
> td_type = build_pointer_type (td_type);
>- loc = LOCATION_LOCUS (loc);
>
> /* Create the structure type. */
> ret = make_node (RECORD_TYPE);
>- if (loc != UNKNOWN_LOCATION)
>+ if (ploc != NULL)
> {
>+ loc = LOCATION_LOCUS (*ploc);
> fields[i] = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
> ubsan_source_location_type ());
> DECL_CONTEXT (fields[i]) = ret;
>@@ -481,7 +485,7 @@ ubsan_create_data (const char *name, loc
> tree ctor = build_constructor (ret, v);
>
> /* If desirable, set the __ubsan_source_location element. */
>- if (loc != UNKNOWN_LOCATION)
>+ if (ploc != NULL)
> CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, ubsan_source_location (loc));
>
> size_t nelts = vec_safe_length (saved_args);
>@@ -513,7 +517,7 @@ tree
> ubsan_instrument_unreachable (location_t loc)
> {
> initialize_sanitizer_builtins ();
>- tree data = ubsan_create_data ("__ubsan_unreachable_data", loc,
>NULL,
>+ tree data = ubsan_create_data ("__ubsan_unreachable_data", &loc,
>NULL,
> NULL_TREE);
>tree t = builtin_decl_explicit
>(BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE);
>return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc,
>data));
>@@ -583,7 +587,7 @@ ubsan_expand_null_ifn (gimple_stmt_itera
> const struct ubsan_mismatch_data m
> = { build_zero_cst (pointer_sized_int_node), ckind };
> tree data = ubsan_create_data ("__ubsan_null_data",
>- loc, &m,
>+ &loc, &m,
> ubsan_type_descriptor (TREE_TYPE (ptr), true),
> NULL_TREE);
> data = build_fold_addr_expr_loc (loc, data);
>@@ -658,7 +662,7 @@ tree
>ubsan_build_overflow_builtin (tree_code code, location_t loc, tree
>lhstype,
> tree op0, tree op1)
> {
>- tree data = ubsan_create_data ("__ubsan_overflow_data", loc, NULL,
>+ tree data = ubsan_create_data ("__ubsan_overflow_data", &loc, NULL,
> ubsan_type_descriptor (lhstype, false),
> NULL_TREE);
> enum built_in_function fn_code;
>@@ -841,7 +845,7 @@ instrument_bool_enum_load (gimple_stmt_i
> update_stmt (stmt);
>
> tree data = ubsan_create_data ("__ubsan_invalid_value_data",
>- loc, NULL,
>+ &loc, NULL,
> ubsan_type_descriptor (type, false),
> NULL_TREE);
> data = build_fold_addr_expr_loc (loc, data);
>--- gcc/c-family/c-ubsan.c.jj 2014-01-03 11:40:29.000000000 +0100
>+++ gcc/c-family/c-ubsan.c 2014-03-26 11:22:12.105464655 +0100
>@@ -73,7 +73,7 @@ ubsan_instrument_division (location_t lo
> make sure it gets evaluated before the condition. */
> t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
> tree data = ubsan_create_data ("__ubsan_overflow_data",
>- loc, NULL,
>+ &loc, NULL,
> ubsan_type_descriptor (type, false),
> NULL_TREE);
> data = build_fold_addr_expr_loc (loc, data);
>@@ -142,7 +142,7 @@ ubsan_instrument_shift (location_t loc,
> make sure it gets evaluated before the condition. */
> t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
> tree data = ubsan_create_data ("__ubsan_shift_data",
>- loc, NULL,
>+ &loc, NULL,
> ubsan_type_descriptor (type0, false),
> ubsan_type_descriptor (type1, false),
> NULL_TREE);
>@@ -169,7 +169,7 @@ ubsan_instrument_vla (location_t loc, tr
>
>t = fold_build2 (LE_EXPR, boolean_type_node, size, build_int_cst (type,
>0));
> tree data = ubsan_create_data ("__ubsan_vla_data",
>- loc, NULL,
>+ &loc, NULL,
> ubsan_type_descriptor (type, false),
> NULL_TREE);
> data = build_fold_addr_expr_loc (loc, data);
>@@ -185,7 +185,7 @@ ubsan_instrument_vla (location_t loc, tr
> tree
> ubsan_instrument_return (location_t loc)
> {
>- tree data = ubsan_create_data ("__ubsan_missing_return_data", loc,
>+ tree data = ubsan_create_data ("__ubsan_missing_return_data", &loc,
> NULL, NULL_TREE);
> tree t = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_MISSING_RETURN);
>return build_call_expr_loc (loc, t, 1, build_fold_addr_expr_loc (loc,
>data));
>
>
> Jakub