| Message ID | ZfBph6aI+23HvYij@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | gimple-iterator, ubsan, v3: Fix ICE during instrumentation of returns_twice calls [PR112709] | expand |
On Tue, 12 Mar 2024, Jakub Jelinek wrote: > On Tue, Mar 12, 2024 at 02:31:28PM +0100, Richard Biener wrote: > > Ah, yeah, I see :/ > > > > > So, the intention of edge_before_returns_twice_call is just that > > > it in the common case just finds the non-EDGE_ABNORMAL edge if there is one, > > > if there isn't just one, it adjusts the IL such that there is just one. > > > And then the next step is to handle that case. > > > > So I guess the updated patch is OK then. > > For the naming thing, another variant would be to export > > void > gsi_safe_insert_before (gimple_stmt_iterator *iter, gimple *g) > { > gimple *stmt = gsi_stmt (*iter); > if (stmt > && is_gimple_call (stmt) > && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0) > gsi_insert_before_returns_twice_call (gsi_bb (*iter), g); > else > gsi_insert_before (iter, g, GSI_SAME_STMT); > } > > /* Similarly for sequence SEQ. */ > > void > gsi_safe_insert_seq_before (gimple_stmt_iterator *iter, gimple_seq seq) > ... > > and inline the gsi_insert_*before_returns_twice_call calls by hand > in there. > Then asan.cc/ubsan.cc wouldn't need to define those functions. > I could even outline the updating of SSA_NAMEs on a single statement > into a helper inline function. > > The patch is even shorter then (the asan patch as well). > > Tested again with make check-gcc check-g++ RUNTESTFLAGS='ubsan.exp asan.exp' I like this more, thus OK. Richard. > 2024-03-12 Jakub Jelinek <jakub@redhat.com> > > PR sanitizer/112709 > * gimple-iterator.h (gsi_safe_insert_before, > gsi_safe_insert_seq_before): Declare. > * gimple-iterator.cc: Include gimplify.h. > (edge_before_returns_twice_call, adjust_before_returns_twice_call, > gsi_safe_insert_before, gsi_safe_insert_seq_before): New functions. > * ubsan.cc (instrument_mem_ref, instrument_pointer_overflow, > instrument_nonnull_arg, instrument_nonnull_return): Use > gsi_safe_insert_before instead of gsi_insert_before. > (maybe_instrument_pointer_overflow): Use force_gimple_operand, > gimple_seq_add_seq_without_update and gsi_safe_insert_seq_before > instead of force_gimple_operand_gsi. > (instrument_object_size): Likewise. Use gsi_safe_insert_before > instead of gsi_insert_before. > > * gcc.dg/ubsan/pr112709-1.c: New test. > * gcc.dg/ubsan/pr112709-2.c: New test. > > --- gcc/gimple-iterator.h.jj 2024-03-12 10:15:41.253529859 +0100 > +++ gcc/gimple-iterator.h 2024-03-12 15:10:23.594845422 +0100 > @@ -93,6 +93,8 @@ extern void gsi_insert_on_edge (edge, gi > extern void gsi_insert_seq_on_edge (edge, gimple_seq); > extern basic_block gsi_insert_on_edge_immediate (edge, gimple *); > extern basic_block gsi_insert_seq_on_edge_immediate (edge, gimple_seq); > +extern void gsi_safe_insert_before (gimple_stmt_iterator *, gimple *); > +extern void gsi_safe_insert_seq_before (gimple_stmt_iterator *, gimple_seq); > extern void gsi_commit_edge_inserts (void); > extern void gsi_commit_one_edge_insert (edge, basic_block *); > extern gphi_iterator gsi_start_phis (basic_block); > --- gcc/gimple-iterator.cc.jj 2024-03-12 10:15:41.209530471 +0100 > +++ gcc/gimple-iterator.cc 2024-03-12 15:29:17.814171376 +0100 > @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. > #include "tree-cfg.h" > #include "tree-ssa.h" > #include "value-prof.h" > +#include "gimplify.h" > > > /* Mark the statement STMT as modified, and update it. */ > @@ -944,3 +945,137 @@ gsi_start_phis (basic_block bb) > > return i; > } > + > +/* Helper function for gsi_safe_insert_before and gsi_safe_insert_seq_before. > + Find edge to insert statements before returns_twice call at the start of BB, > + if there isn't just one, split the bb and adjust PHIs to ensure that. */ > + > +static edge > +edge_before_returns_twice_call (basic_block bb) > +{ > + gimple_stmt_iterator gsi = gsi_start_nondebug_bb (bb); > + gcc_checking_assert (is_gimple_call (gsi_stmt (gsi)) > + && (gimple_call_flags (gsi_stmt (gsi)) > + & ECF_RETURNS_TWICE) != 0); > + edge_iterator ei; > + edge e, ad_edge = NULL, other_edge = NULL; > + bool split = false; > + FOR_EACH_EDGE (e, ei, bb->preds) > + { > + if ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) == EDGE_ABNORMAL) > + { > + gimple_stmt_iterator gsi > + = gsi_start_nondebug_after_labels_bb (e->src); > + gimple *ad = gsi_stmt (gsi); > + if (ad && gimple_call_internal_p (ad, IFN_ABNORMAL_DISPATCHER)) > + { > + gcc_checking_assert (ad_edge == NULL); > + ad_edge = e; > + continue; > + } > + } > + if (other_edge || e->flags & (EDGE_ABNORMAL | EDGE_EH)) > + split = true; > + other_edge = e; > + } > + gcc_checking_assert (ad_edge); > + if (other_edge == NULL) > + split = true; > + if (split) > + { > + other_edge = split_block_after_labels (bb); > + e = make_edge (ad_edge->src, other_edge->dest, EDGE_ABNORMAL); > + for (gphi_iterator gsi = gsi_start_phis (other_edge->src); > + !gsi_end_p (gsi); gsi_next (&gsi)) > + { > + gphi *phi = gsi.phi (); > + tree lhs = gimple_phi_result (phi); > + tree new_lhs = copy_ssa_name (lhs); > + gimple_phi_set_result (phi, new_lhs); > + gphi *new_phi = create_phi_node (lhs, other_edge->dest); > + add_phi_arg (new_phi, new_lhs, other_edge, UNKNOWN_LOCATION); > + add_phi_arg (new_phi, gimple_phi_arg_def_from_edge (phi, ad_edge), > + e, gimple_phi_arg_location_from_edge (phi, ad_edge)); > + } > + remove_edge (ad_edge); > + } > + return other_edge; > +} > + > +/* Helper function for gsi_safe_insert_before and gsi_safe_insert_seq_before. > + Replace SSA_NAME uses in G if they are PHI results of PHIs on E->dest > + bb with the corresponding PHI argument from E edge. */ > + > +static void > +adjust_before_returns_twice_call (edge e, gimple *g) > +{ > + use_operand_p use_p; > + ssa_op_iter iter; > + bool m = false; > + FOR_EACH_SSA_USE_OPERAND (use_p, g, iter, SSA_OP_USE) > + { > + tree s = USE_FROM_PTR (use_p); > + if (SSA_NAME_DEF_STMT (s) > + && gimple_code (SSA_NAME_DEF_STMT (s)) == GIMPLE_PHI > + && gimple_bb (SSA_NAME_DEF_STMT (s)) == e->dest) > + { > + tree r = gimple_phi_arg_def_from_edge (SSA_NAME_DEF_STMT (s), e); > + SET_USE (use_p, unshare_expr (r)); > + m = true; > + } > + } > + if (m) > + update_stmt (g); > +} > + > +/* Insert G stmt before ITER and keep ITER pointing to the same statement > + as before. If ITER is a returns_twice call, insert it on an appropriate > + edge instead. */ > + > +void > +gsi_safe_insert_before (gimple_stmt_iterator *iter, gimple *g) > +{ > + gimple *stmt = gsi_stmt (*iter); > + if (stmt > + && is_gimple_call (stmt) > + && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0) > + { > + edge e = edge_before_returns_twice_call (gsi_bb (*iter)); > + basic_block new_bb = gsi_insert_on_edge_immediate (e, g); > + if (new_bb) > + e = single_succ_edge (new_bb); > + adjust_before_returns_twice_call (e, g); > + } > + else > + gsi_insert_before (iter, g, GSI_SAME_STMT); > +} > + > +/* Similarly for sequence SEQ. */ > + > +void > +gsi_safe_insert_seq_before (gimple_stmt_iterator *iter, gimple_seq seq) > +{ > + if (gimple_seq_empty_p (seq)) > + return; > + gimple *stmt = gsi_stmt (*iter); > + if (stmt > + && is_gimple_call (stmt) > + && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0) > + { > + edge e = edge_before_returns_twice_call (gsi_bb (*iter)); > + gimple *f = gimple_seq_first_stmt (seq); > + gimple *l = gimple_seq_last_stmt (seq); > + basic_block new_bb = gsi_insert_seq_on_edge_immediate (e, seq); > + if (new_bb) > + e = single_succ_edge (new_bb); > + for (gimple_stmt_iterator gsi = gsi_for_stmt (f); ; gsi_next (&gsi)) > + { > + gimple *g = gsi_stmt (gsi); > + adjust_before_returns_twice_call (e, g); > + if (g == l) > + break; > + } > + } > + else > + gsi_insert_seq_before (iter, seq, GSI_SAME_STMT); > +} > --- gcc/ubsan.cc.jj 2024-03-12 10:15:41.306529121 +0100 > +++ gcc/ubsan.cc 2024-03-12 15:08:17.073594937 +0100 > @@ -1458,7 +1458,7 @@ instrument_mem_ref (tree mem, tree base, > tree alignt = build_int_cst (pointer_sized_int_node, align); > gcall *g = gimple_build_call_internal (IFN_UBSAN_NULL, 3, t, kind, alignt); > gimple_set_location (g, gimple_location (gsi_stmt (*iter))); > - gsi_insert_before (iter, g, GSI_SAME_STMT); > + gsi_safe_insert_before (iter, g); > } > > /* Perform the pointer instrumentation. */ > @@ -1485,7 +1485,7 @@ instrument_pointer_overflow (gimple_stmt > return; > gcall *g = gimple_build_call_internal (IFN_UBSAN_PTR, 2, ptr, off); > gimple_set_location (g, gimple_location (gsi_stmt (*gsi))); > - gsi_insert_before (gsi, g, GSI_SAME_STMT); > + gsi_safe_insert_before (gsi, g); > } > > /* Instrument pointer arithmetics if any. */ > @@ -1577,10 +1577,11 @@ maybe_instrument_pointer_overflow (gimpl > else > t = fold_convert (sizetype, moff); > } > - t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true, > - GSI_SAME_STMT); > - base_addr = force_gimple_operand_gsi (gsi, base_addr, true, NULL_TREE, true, > - GSI_SAME_STMT); > + gimple_seq seq, this_seq; > + t = force_gimple_operand (t, &seq, true, NULL_TREE); > + base_addr = force_gimple_operand (base_addr, &this_seq, true, NULL_TREE); > + gimple_seq_add_seq_without_update (&seq, this_seq); > + gsi_safe_insert_seq_before (gsi, seq); > instrument_pointer_overflow (gsi, base_addr, t); > } > > @@ -2035,7 +2036,7 @@ instrument_nonnull_arg (gimple_stmt_iter > { > g = gimple_build_assign (make_ssa_name (TREE_TYPE (arg)), arg); > gimple_set_location (g, loc[0]); > - gsi_insert_before (gsi, g, GSI_SAME_STMT); > + gsi_safe_insert_before (gsi, g); > arg = gimple_assign_lhs (g); > } > > @@ -2068,7 +2069,7 @@ instrument_nonnull_arg (gimple_stmt_iter > g = gimple_build_call (fn, 1, data); > } > gimple_set_location (g, loc[0]); > - gsi_insert_before (gsi, g, GSI_SAME_STMT); > + gsi_safe_insert_before (gsi, g); > ubsan_create_edge (g); > } > *gsi = gsi_for_stmt (stmt); > @@ -2124,7 +2125,7 @@ instrument_nonnull_return (gimple_stmt_i > g = gimple_build_call (fn, 2, data, data2); > } > gimple_set_location (g, loc[0]); > - gsi_insert_before (gsi, g, GSI_SAME_STMT); > + gsi_safe_insert_before (gsi, g); > ubsan_create_edge (g); > *gsi = gsi_for_stmt (stmt); > } > @@ -2231,6 +2232,7 @@ instrument_object_size (gimple_stmt_iter > tree sizet; > tree base_addr = base; > gimple *bos_stmt = NULL; > + gimple_seq seq = NULL; > if (decl_p) > base_addr = build1 (ADDR_EXPR, > build_pointer_type (TREE_TYPE (base)), base); > @@ -2244,19 +2246,12 @@ instrument_object_size (gimple_stmt_iter > sizet = builtin_decl_explicit (BUILT_IN_DYNAMIC_OBJECT_SIZE); > 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); > + sizet = force_gimple_operand (sizet, &seq, false, NULL_TREE); > /* If the call above didn't end up being an integer constant, go one > statement back and get the __builtin_object_size stmt. Save it, > we might need it later. */ > if (SSA_VAR_P (sizet)) > - { > - gsi_prev (gsi); > - bos_stmt = gsi_stmt (*gsi); > - > - /* Move on to where we were. */ > - gsi_next (gsi); > - } > + bos_stmt = gsi_stmt (gsi_last (seq)); > } > else > return; > @@ -2298,21 +2293,24 @@ instrument_object_size (gimple_stmt_iter > && !TREE_ADDRESSABLE (base)) > mark_addressable (base); > > + /* We have to emit the check. */ > + gimple_seq this_seq; > + t = force_gimple_operand (t, &this_seq, true, NULL_TREE); > + gimple_seq_add_seq_without_update (&seq, this_seq); > + ptr = force_gimple_operand (ptr, &this_seq, true, NULL_TREE); > + gimple_seq_add_seq_without_update (&seq, this_seq); > + gsi_safe_insert_seq_before (gsi, seq); > + > if (bos_stmt > && gimple_call_builtin_p (bos_stmt, BUILT_IN_DYNAMIC_OBJECT_SIZE)) > ubsan_create_edge (bos_stmt); > > - /* We have to emit the check. */ > - t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true, > - GSI_SAME_STMT); > - ptr = force_gimple_operand_gsi (gsi, ptr, true, NULL_TREE, true, > - GSI_SAME_STMT); > tree ckind = build_int_cst (unsigned_char_type_node, > is_lhs ? UBSAN_STORE_OF : UBSAN_LOAD_OF); > gimple *g = gimple_build_call_internal (IFN_UBSAN_OBJECT_SIZE, 4, > ptr, t, sizet, ckind); > gimple_set_location (g, loc); > - gsi_insert_before (gsi, g, GSI_SAME_STMT); > + gsi_safe_insert_before (gsi, g); > } > > /* Instrument values passed to builtin functions. */ > --- gcc/testsuite/gcc.dg/ubsan/pr112709-1.c.jj 2024-03-12 12:34:56.027880687 +0100 > +++ gcc/testsuite/gcc.dg/ubsan/pr112709-1.c 2024-03-12 12:57:58.993687023 +0100 > @@ -0,0 +1,64 @@ > +/* PR sanitizer/112709 */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=undefined -O2" } */ > + > +struct S { char c[1024]; }; > +int foo (int); > + > +__attribute__((returns_twice, noipa)) struct S > +bar (int x) > +{ > + (void) x; > + struct S s = {}; > + s.c[42] = 42; > + return s; > +} > + > +void > +baz (struct S *p) > +{ > + foo (1); > + *p = bar (0); > +} > + > +void > +qux (int x, struct S *p) > +{ > + if (x == 25) > + x = foo (2); > + else if (x == 42) > + x = foo (foo (3)); > + *p = bar (x); > +} > + > +void > +corge (int x, struct S *p) > +{ > + void *q[] = { &&l1, &&l2, &&l3, &&l3 }; > + if (x == 25) > + { > + l1: > + x = foo (2); > + } > + else if (x == 42) > + { > + l2: > + x = foo (foo (3)); > + } > +l3: > + *p = bar (x); > + if (x < 4) > + goto *q[x & 3]; > +} > + > +void > +freddy (int x, struct S *p) > +{ > + *p = bar (x); > + ++p; > + if (x == 25) > + x = foo (2); > + else if (x == 42) > + x = foo (foo (3)); > + *p = bar (x); > +} > --- gcc/testsuite/gcc.dg/ubsan/pr112709-2.c.jj 2024-03-12 12:34:56.027880687 +0100 > +++ gcc/testsuite/gcc.dg/ubsan/pr112709-2.c 2024-03-12 12:58:13.305488706 +0100 > @@ -0,0 +1,62 @@ > +/* PR sanitizer/112709 */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=undefined -O2" } */ > + > +struct S { char c[1024]; } *p; > +int foo (int); > + > +__attribute__((returns_twice, noipa)) int > +bar (struct S x) > +{ > + (void) x.c[0]; > + return 0; > +} > + > +void > +baz (int *y) > +{ > + foo (1); > + *y = bar (*p); > +} > + > +void > +qux (int x, int *y) > +{ > + if (x == 25) > + x = foo (2); > + else if (x == 42) > + x = foo (foo (3)); > + *y = bar (*p); > +} > + > +void > +corge (int x, int *y) > +{ > + void *q[] = { &&l1, &&l2, &&l3, &&l3 }; > + if (x == 25) > + { > + l1: > + x = foo (2); > + } > + else if (x == 42) > + { > + l2: > + x = foo (foo (3)); > + } > +l3: > + *y = bar (*p); > + if (x < 4) > + goto *q[x & 3]; > +} > + > +void > +freddy (int x, int *y, struct S *p) > +{ > + bar (*p); > + ++p; > + if (x == 25) > + x = foo (2); > + else if (x == 42) > + x = foo (foo (3)); > + *y = bar (*p); > +} > > > Jakub > >
--- gcc/gimple-iterator.h.jj 2024-03-12 10:15:41.253529859 +0100 +++ gcc/gimple-iterator.h 2024-03-12 15:10:23.594845422 +0100 @@ -93,6 +93,8 @@ extern void gsi_insert_on_edge (edge, gi extern void gsi_insert_seq_on_edge (edge, gimple_seq); extern basic_block gsi_insert_on_edge_immediate (edge, gimple *); extern basic_block gsi_insert_seq_on_edge_immediate (edge, gimple_seq); +extern void gsi_safe_insert_before (gimple_stmt_iterator *, gimple *); +extern void gsi_safe_insert_seq_before (gimple_stmt_iterator *, gimple_seq); extern void gsi_commit_edge_inserts (void); extern void gsi_commit_one_edge_insert (edge, basic_block *); extern gphi_iterator gsi_start_phis (basic_block); --- gcc/gimple-iterator.cc.jj 2024-03-12 10:15:41.209530471 +0100 +++ gcc/gimple-iterator.cc 2024-03-12 15:29:17.814171376 +0100 @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. #include "tree-cfg.h" #include "tree-ssa.h" #include "value-prof.h" +#include "gimplify.h" /* Mark the statement STMT as modified, and update it. */ @@ -944,3 +945,137 @@ gsi_start_phis (basic_block bb) return i; } + +/* Helper function for gsi_safe_insert_before and gsi_safe_insert_seq_before. + Find edge to insert statements before returns_twice call at the start of BB, + if there isn't just one, split the bb and adjust PHIs to ensure that. */ + +static edge +edge_before_returns_twice_call (basic_block bb) +{ + gimple_stmt_iterator gsi = gsi_start_nondebug_bb (bb); + gcc_checking_assert (is_gimple_call (gsi_stmt (gsi)) + && (gimple_call_flags (gsi_stmt (gsi)) + & ECF_RETURNS_TWICE) != 0); + edge_iterator ei; + edge e, ad_edge = NULL, other_edge = NULL; + bool split = false; + FOR_EACH_EDGE (e, ei, bb->preds) + { + if ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) == EDGE_ABNORMAL) + { + gimple_stmt_iterator gsi + = gsi_start_nondebug_after_labels_bb (e->src); + gimple *ad = gsi_stmt (gsi); + if (ad && gimple_call_internal_p (ad, IFN_ABNORMAL_DISPATCHER)) + { + gcc_checking_assert (ad_edge == NULL); + ad_edge = e; + continue; + } + } + if (other_edge || e->flags & (EDGE_ABNORMAL | EDGE_EH)) + split = true; + other_edge = e; + } + gcc_checking_assert (ad_edge); + if (other_edge == NULL) + split = true; + if (split) + { + other_edge = split_block_after_labels (bb); + e = make_edge (ad_edge->src, other_edge->dest, EDGE_ABNORMAL); + for (gphi_iterator gsi = gsi_start_phis (other_edge->src); + !gsi_end_p (gsi); gsi_next (&gsi)) + { + gphi *phi = gsi.phi (); + tree lhs = gimple_phi_result (phi); + tree new_lhs = copy_ssa_name (lhs); + gimple_phi_set_result (phi, new_lhs); + gphi *new_phi = create_phi_node (lhs, other_edge->dest); + add_phi_arg (new_phi, new_lhs, other_edge, UNKNOWN_LOCATION); + add_phi_arg (new_phi, gimple_phi_arg_def_from_edge (phi, ad_edge), + e, gimple_phi_arg_location_from_edge (phi, ad_edge)); + } + remove_edge (ad_edge); + } + return other_edge; +} + +/* Helper function for gsi_safe_insert_before and gsi_safe_insert_seq_before. + Replace SSA_NAME uses in G if they are PHI results of PHIs on E->dest + bb with the corresponding PHI argument from E edge. */ + +static void +adjust_before_returns_twice_call (edge e, gimple *g) +{ + use_operand_p use_p; + ssa_op_iter iter; + bool m = false; + FOR_EACH_SSA_USE_OPERAND (use_p, g, iter, SSA_OP_USE) + { + tree s = USE_FROM_PTR (use_p); + if (SSA_NAME_DEF_STMT (s) + && gimple_code (SSA_NAME_DEF_STMT (s)) == GIMPLE_PHI + && gimple_bb (SSA_NAME_DEF_STMT (s)) == e->dest) + { + tree r = gimple_phi_arg_def_from_edge (SSA_NAME_DEF_STMT (s), e); + SET_USE (use_p, unshare_expr (r)); + m = true; + } + } + if (m) + update_stmt (g); +} + +/* Insert G stmt before ITER and keep ITER pointing to the same statement + as before. If ITER is a returns_twice call, insert it on an appropriate + edge instead. */ + +void +gsi_safe_insert_before (gimple_stmt_iterator *iter, gimple *g) +{ + gimple *stmt = gsi_stmt (*iter); + if (stmt + && is_gimple_call (stmt) + && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0) + { + edge e = edge_before_returns_twice_call (gsi_bb (*iter)); + basic_block new_bb = gsi_insert_on_edge_immediate (e, g); + if (new_bb) + e = single_succ_edge (new_bb); + adjust_before_returns_twice_call (e, g); + } + else + gsi_insert_before (iter, g, GSI_SAME_STMT); +} + +/* Similarly for sequence SEQ. */ + +void +gsi_safe_insert_seq_before (gimple_stmt_iterator *iter, gimple_seq seq) +{ + if (gimple_seq_empty_p (seq)) + return; + gimple *stmt = gsi_stmt (*iter); + if (stmt + && is_gimple_call (stmt) + && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0) + { + edge e = edge_before_returns_twice_call (gsi_bb (*iter)); + gimple *f = gimple_seq_first_stmt (seq); + gimple *l = gimple_seq_last_stmt (seq); + basic_block new_bb = gsi_insert_seq_on_edge_immediate (e, seq); + if (new_bb) + e = single_succ_edge (new_bb); + for (gimple_stmt_iterator gsi = gsi_for_stmt (f); ; gsi_next (&gsi)) + { + gimple *g = gsi_stmt (gsi); + adjust_before_returns_twice_call (e, g); + if (g == l) + break; + } + } + else + gsi_insert_seq_before (iter, seq, GSI_SAME_STMT); +} --- gcc/ubsan.cc.jj 2024-03-12 10:15:41.306529121 +0100 +++ gcc/ubsan.cc 2024-03-12 15:08:17.073594937 +0100 @@ -1458,7 +1458,7 @@ instrument_mem_ref (tree mem, tree base, tree alignt = build_int_cst (pointer_sized_int_node, align); gcall *g = gimple_build_call_internal (IFN_UBSAN_NULL, 3, t, kind, alignt); gimple_set_location (g, gimple_location (gsi_stmt (*iter))); - gsi_insert_before (iter, g, GSI_SAME_STMT); + gsi_safe_insert_before (iter, g); } /* Perform the pointer instrumentation. */ @@ -1485,7 +1485,7 @@ instrument_pointer_overflow (gimple_stmt return; gcall *g = gimple_build_call_internal (IFN_UBSAN_PTR, 2, ptr, off); gimple_set_location (g, gimple_location (gsi_stmt (*gsi))); - gsi_insert_before (gsi, g, GSI_SAME_STMT); + gsi_safe_insert_before (gsi, g); } /* Instrument pointer arithmetics if any. */ @@ -1577,10 +1577,11 @@ maybe_instrument_pointer_overflow (gimpl else t = fold_convert (sizetype, moff); } - t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true, - GSI_SAME_STMT); - base_addr = force_gimple_operand_gsi (gsi, base_addr, true, NULL_TREE, true, - GSI_SAME_STMT); + gimple_seq seq, this_seq; + t = force_gimple_operand (t, &seq, true, NULL_TREE); + base_addr = force_gimple_operand (base_addr, &this_seq, true, NULL_TREE); + gimple_seq_add_seq_without_update (&seq, this_seq); + gsi_safe_insert_seq_before (gsi, seq); instrument_pointer_overflow (gsi, base_addr, t); } @@ -2035,7 +2036,7 @@ instrument_nonnull_arg (gimple_stmt_iter { g = gimple_build_assign (make_ssa_name (TREE_TYPE (arg)), arg); gimple_set_location (g, loc[0]); - gsi_insert_before (gsi, g, GSI_SAME_STMT); + gsi_safe_insert_before (gsi, g); arg = gimple_assign_lhs (g); } @@ -2068,7 +2069,7 @@ instrument_nonnull_arg (gimple_stmt_iter g = gimple_build_call (fn, 1, data); } gimple_set_location (g, loc[0]); - gsi_insert_before (gsi, g, GSI_SAME_STMT); + gsi_safe_insert_before (gsi, g); ubsan_create_edge (g); } *gsi = gsi_for_stmt (stmt); @@ -2124,7 +2125,7 @@ instrument_nonnull_return (gimple_stmt_i g = gimple_build_call (fn, 2, data, data2); } gimple_set_location (g, loc[0]); - gsi_insert_before (gsi, g, GSI_SAME_STMT); + gsi_safe_insert_before (gsi, g); ubsan_create_edge (g); *gsi = gsi_for_stmt (stmt); } @@ -2231,6 +2232,7 @@ instrument_object_size (gimple_stmt_iter tree sizet; tree base_addr = base; gimple *bos_stmt = NULL; + gimple_seq seq = NULL; if (decl_p) base_addr = build1 (ADDR_EXPR, build_pointer_type (TREE_TYPE (base)), base); @@ -2244,19 +2246,12 @@ instrument_object_size (gimple_stmt_iter sizet = builtin_decl_explicit (BUILT_IN_DYNAMIC_OBJECT_SIZE); 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); + sizet = force_gimple_operand (sizet, &seq, false, NULL_TREE); /* If the call above didn't end up being an integer constant, go one statement back and get the __builtin_object_size stmt. Save it, we might need it later. */ if (SSA_VAR_P (sizet)) - { - gsi_prev (gsi); - bos_stmt = gsi_stmt (*gsi); - - /* Move on to where we were. */ - gsi_next (gsi); - } + bos_stmt = gsi_stmt (gsi_last (seq)); } else return; @@ -2298,21 +2293,24 @@ instrument_object_size (gimple_stmt_iter && !TREE_ADDRESSABLE (base)) mark_addressable (base); + /* We have to emit the check. */ + gimple_seq this_seq; + t = force_gimple_operand (t, &this_seq, true, NULL_TREE); + gimple_seq_add_seq_without_update (&seq, this_seq); + ptr = force_gimple_operand (ptr, &this_seq, true, NULL_TREE); + gimple_seq_add_seq_without_update (&seq, this_seq); + gsi_safe_insert_seq_before (gsi, seq); + if (bos_stmt && gimple_call_builtin_p (bos_stmt, BUILT_IN_DYNAMIC_OBJECT_SIZE)) ubsan_create_edge (bos_stmt); - /* We have to emit the check. */ - t = force_gimple_operand_gsi (gsi, t, true, NULL_TREE, true, - GSI_SAME_STMT); - ptr = force_gimple_operand_gsi (gsi, ptr, true, NULL_TREE, true, - GSI_SAME_STMT); tree ckind = build_int_cst (unsigned_char_type_node, is_lhs ? UBSAN_STORE_OF : UBSAN_LOAD_OF); gimple *g = gimple_build_call_internal (IFN_UBSAN_OBJECT_SIZE, 4, ptr, t, sizet, ckind); gimple_set_location (g, loc); - gsi_insert_before (gsi, g, GSI_SAME_STMT); + gsi_safe_insert_before (gsi, g); } /* Instrument values passed to builtin functions. */ --- gcc/testsuite/gcc.dg/ubsan/pr112709-1.c.jj 2024-03-12 12:34:56.027880687 +0100 +++ gcc/testsuite/gcc.dg/ubsan/pr112709-1.c 2024-03-12 12:57:58.993687023 +0100 @@ -0,0 +1,64 @@ +/* PR sanitizer/112709 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined -O2" } */ + +struct S { char c[1024]; }; +int foo (int); + +__attribute__((returns_twice, noipa)) struct S +bar (int x) +{ + (void) x; + struct S s = {}; + s.c[42] = 42; + return s; +} + +void +baz (struct S *p) +{ + foo (1); + *p = bar (0); +} + +void +qux (int x, struct S *p) +{ + if (x == 25) + x = foo (2); + else if (x == 42) + x = foo (foo (3)); + *p = bar (x); +} + +void +corge (int x, struct S *p) +{ + void *q[] = { &&l1, &&l2, &&l3, &&l3 }; + if (x == 25) + { + l1: + x = foo (2); + } + else if (x == 42) + { + l2: + x = foo (foo (3)); + } +l3: + *p = bar (x); + if (x < 4) + goto *q[x & 3]; +} + +void +freddy (int x, struct S *p) +{ + *p = bar (x); + ++p; + if (x == 25) + x = foo (2); + else if (x == 42) + x = foo (foo (3)); + *p = bar (x); +} --- gcc/testsuite/gcc.dg/ubsan/pr112709-2.c.jj 2024-03-12 12:34:56.027880687 +0100 +++ gcc/testsuite/gcc.dg/ubsan/pr112709-2.c 2024-03-12 12:58:13.305488706 +0100 @@ -0,0 +1,62 @@ +/* PR sanitizer/112709 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined -O2" } */ + +struct S { char c[1024]; } *p; +int foo (int); + +__attribute__((returns_twice, noipa)) int +bar (struct S x) +{ + (void) x.c[0]; + return 0; +} + +void +baz (int *y) +{ + foo (1); + *y = bar (*p); +} + +void +qux (int x, int *y) +{ + if (x == 25) + x = foo (2); + else if (x == 42) + x = foo (foo (3)); + *y = bar (*p); +} + +void +corge (int x, int *y) +{ + void *q[] = { &&l1, &&l2, &&l3, &&l3 }; + if (x == 25) + { + l1: + x = foo (2); + } + else if (x == 42) + { + l2: + x = foo (foo (3)); + } +l3: + *y = bar (*p); + if (x < 4) + goto *q[x & 3]; +} + +void +freddy (int x, int *y, struct S *p) +{ + bar (*p); + ++p; + if (x == 25) + x = foo (2); + else if (x == 42) + x = foo (foo (3)); + *y = bar (*p); +}