Message ID | e04b1ac8-e3ff-b13f-4006-bf961af6d2ea@suse.cz |
---|---|
State | New |
Headers | show |
Hi! On Fri, Nov 04, 2016 at 10:17:31AM +0100, Martin Liška wrote: > diff --git a/gcc/gimplify.c b/gcc/gimplify.c > index 813777d..86ce793 100644 > --- a/gcc/gimplify.c > +++ b/gcc/gimplify.c > @@ -1678,7 +1678,9 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p, > worse location info. */ > if (gimple_try_eval (stmt) == NULL) > { > - wi->info = stmt; > + gimple_stmt_iterator *it = XNEW (gimple_stmt_iterator); > + memcpy (it, gsi_p, sizeof (gimple_stmt_iterator)); That would be cleaner as *it = *gsi_p; That set, I fail to see 1) the need to use a gsi pointer in wi->info compared to stmt itself, you can gsi_for_stmt cheaply at any time 2) why is anything done about this in warn_switch_unreachable_r - the problem isn't related to this warning IMHO. Even switch (x) { case 1: int x; x = 6; ptr = &x; break; case 2: ptr = &x; *ptr = 7; break; } has the same issue and there is no switch unreachable code there, but you still want for -fsanitize-use-after-scope pretend it is actually: x_tmp = x; { int x; switch (x_tmp) { case 1: x = 6; ptr = &x; break; case 2: ptr = &x; *ptr = 7; break; } } and put ASAN_MARK unpoisoning before GIMPLE_SWITCH. Jakub
On 11/04/2016 10:32 AM, Jakub Jelinek wrote: > Hi! > > On Fri, Nov 04, 2016 at 10:17:31AM +0100, Martin Liška wrote: >> diff --git a/gcc/gimplify.c b/gcc/gimplify.c >> index 813777d..86ce793 100644 >> --- a/gcc/gimplify.c >> +++ b/gcc/gimplify.c >> @@ -1678,7 +1678,9 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p, >> worse location info. */ >> if (gimple_try_eval (stmt) == NULL) >> { >> - wi->info = stmt; >> + gimple_stmt_iterator *it = XNEW (gimple_stmt_iterator); >> + memcpy (it, gsi_p, sizeof (gimple_stmt_iterator)); > > That would be cleaner as *it = *gsi_p; I know that it's kind of ugly, but as the gimple stmt is not yet added to a BB, using gsi_for_stmt ICEs: /tmp/use-after-scope-switch.c:12:5: internal compiler error: Segmentation fault switch (argc) ^~~~~~ 0xe16a14 crash_signal ../../gcc/toplev.c:338 0xadf890 bb_seq_addr ../../gcc/gimple.h:1658 0xae01dd gsi_start_bb ../../gcc/gimple-iterator.h:129 0xae111f gsi_for_stmt(gimple*) ../../gcc/gimple-iterator.c:617 > That set, I fail to see > 1) the need to use a gsi pointer in wi->info compared to stmt itself, > you can gsi_for_stmt cheaply at any time > 2) why is anything done about this in warn_switch_unreachable_r > - the problem isn't related to this warning IMHO. Even > switch (x) > { > case 1: > int x; > x = 6; > ptr = &x; > break; > case 2: > ptr = &x; > *ptr = 7; > break; > } > has the same issue and there is no switch unreachable code there, but you > still want for -fsanitize-use-after-scope pretend it is actually: You're right, that's not handled. I'm wondering whether it's always profitable because when you do not reach the case 1, you're not doing a poisoning operation? Martin > x_tmp = x; > { > int x; > switch (x_tmp) > { > case 1: > x = 6; > ptr = &x; > break; > case 2: > ptr = &x; > *ptr = 7; > break; > } > } > and put ASAN_MARK unpoisoning before GIMPLE_SWITCH. > > Jakub >
diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 813777d..86ce793 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1678,7 +1678,9 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p, worse location info. */ if (gimple_try_eval (stmt) == NULL) { - wi->info = stmt; + gimple_stmt_iterator *it = XNEW (gimple_stmt_iterator); + memcpy (it, gsi_p, sizeof (gimple_stmt_iterator)); + wi->info = it; return integer_zero_node; } /* Fall through. */ @@ -1689,9 +1691,18 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p, /* Walk the sub-statements. */ *handled_ops_p = false; break; + case GIMPLE_CALL: + if (gimple_call_internal_p (stmt, IFN_ASAN_MARK)) + { + *handled_ops_p = false; + break; + } + /* Fall through. */ default: /* Save the first "real" statement (not a decl/lexical scope/...). */ - wi->info = stmt; + gimple_stmt_iterator *it = XNEW (gimple_stmt_iterator); + memcpy (it, gsi_p, sizeof (gimple_stmt_iterator)); + wi->info = it; return integer_zero_node; } return NULL_TREE; @@ -1713,7 +1724,11 @@ maybe_warn_switch_unreachable (gimple_seq seq) struct walk_stmt_info wi; memset (&wi, 0, sizeof (wi)); walk_gimple_seq (seq, warn_switch_unreachable_r, NULL, &wi); - gimple *stmt = (gimple *) wi.info; + gimple *stmt = NULL; + gimple_stmt_iterator *gsi = (gimple_stmt_iterator *) wi.info; + if (gsi) + stmt = gsi_stmt (*gsi); + free (wi.info); if (stmt && gimple_code (stmt) != GIMPLE_LABEL) { @@ -1802,6 +1900,8 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p, if (find_label_entry (labels, label)) prev = gsi_stmt (*gsi_p); } + else if (gimple_call_internal_p (gsi_stmt (*gsi_p), IFN_ASAN_MARK)) + ; else prev = gsi_stmt (*gsi_p); gsi_next (gsi_p); @@ -2224,7 +2239,22 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p) switch_stmt = gimple_build_switch (SWITCH_COND (switch_expr), default_case, labels); - gimplify_seq_add_stmt (pre_p, switch_stmt); + + if (asan_sanitize_use_after_scope ()) + { + struct walk_stmt_info wi; + memset (&wi, 0, sizeof (wi)); + walk_gimple_seq (switch_body_seq, warn_switch_unreachable_r, NULL, &wi); + gimple_stmt_iterator *it = (gimple_stmt_iterator *)wi.info; + if (gsi_stmt (*it) == switch_body_seq) + gimplify_seq_add_stmt (pre_p, switch_stmt); + else + gsi_insert_before_without_update (it, switch_stmt, GSI_SAME_STMT); + free (it); + } + else + gimplify_seq_add_stmt (pre_p, switch_stmt); + gimplify_seq_add_seq (pre_p, switch_body_seq); labels.release (); } diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index db72156..150b2ab 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "cfgloop.h" #include "gimple-low.h" +#include "asan.h" /* In some instances a tree and a gimple need to be stored in a same table, i.e. in hash tables. This is a structure to do this. */ @@ -706,6 +707,9 @@ verify_norecord_switch_expr (struct leh_state *state, if (!tf) return; + if (asan_sanitize_use_after_scope ()) + return; + n = gimple_switch_num_labels (switch_expr); for (i = 0; i < n; ++i) -- 2.10.1