| Message ID | 782727c2-9173-24ab-4e4c-07918dc16bf6@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 01, 2016 at 03:47:54PM +0100, Martin Liška wrote: > On 10/27/2016 07:23 PM, Jakub Jelinek wrote: > > Ok for trunk with that change. > > > > Jakub > > Hello. > > I'll commit the patch as soon as following patch would be accepted. The patch > fixes false positives when running asan-bootstrap. What kind of false positives it is for each case? Is it with normal asan-bootstrap (without your -fsanitize-use-after-scope changes), or only with those changes, or only with those changes and -fsanitize-use-after-scope used during bootstrap? > >From b62e4d7ffe659ec338ef83e84ccb572a07264283 Mon Sep 17 00:00:00 2001 > From: marxin <mliska@suse.cz> > Date: Tue, 20 Sep 2016 10:31:25 +0200 > Subject: [PATCH 1/4] Fix ASAN bootstrap uninitialized warning. > > gcc/ChangeLog: > > 2016-09-26 Martin Liska <mliska@suse.cz> > > * ipa-devirt.c (record_targets_from_bases): Initialize a > variable. > * omp-low.c (lower_omp_target): Remove a variable from > scope defined by a switch statement. > * tree-dump.c (dequeue_and_dump): Likewise. > > gcc/java/ChangeLog: > > 2016-09-26 Martin Liska <mliska@suse.cz> > > * mangle.c (mangle_type): Remove a variable from > scope defined by a switch statement. > --- > gcc/ipa-devirt.c | 2 +- > gcc/omp-low.c | 11 ++++------- > gcc/tree-dump.c | 8 +++----- > 3 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c > index 49e2195..5c0ae72 100644 > --- a/gcc/ipa-devirt.c > +++ b/gcc/ipa-devirt.c > @@ -2862,7 +2862,7 @@ record_targets_from_bases (tree otr_type, > { > while (true) > { > - HOST_WIDE_INT pos, size; > + HOST_WIDE_INT pos = 0, size; > tree base_binfo; > tree fld; > > diff --git a/gcc/omp-low.c b/gcc/omp-low.c > index e5b9e4c..62c9e5c 100644 > --- a/gcc/omp-low.c > +++ b/gcc/omp-low.c > @@ -15803,11 +15803,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) > push_gimplify_context (); > fplist = NULL; > > + tree var, x; > for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) > switch (OMP_CLAUSE_CODE (c)) > { > - tree var, x; > - > default: > break; > case OMP_CLAUSE_MAP: > @@ -16066,12 +16065,11 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) > vec_alloc (vkind, map_cnt); > unsigned int map_idx = 0; > > + tree ovar, nc, s, purpose, var, x, type; > + unsigned int talign; > for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) > switch (OMP_CLAUSE_CODE (c)) > { > - tree ovar, nc, s, purpose, var, x, type; > - unsigned int talign; > - > default: > break; > > @@ -16442,10 +16440,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) > if (offloaded || data_region) > { > tree prev = NULL_TREE; > + tree var, x; > for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) > switch (OMP_CLAUSE_CODE (c)) > { > - tree var, x; > default: > break; > case OMP_CLAUSE_FIRSTPRIVATE: > @@ -16594,7 +16592,6 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) > for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) > switch (OMP_CLAUSE_CODE (c)) > { > - tree var; > default: > break; > case OMP_CLAUSE_MAP: > diff --git a/gcc/tree-dump.c b/gcc/tree-dump.c > index df47181..89f72a0 100644 > --- a/gcc/tree-dump.c > +++ b/gcc/tree-dump.c > @@ -420,8 +420,6 @@ dequeue_and_dump (dump_info_p di) > /* Now handle the various kinds of nodes. */ > switch (code) > { > - int i; > - > case IDENTIFIER_NODE: > dump_string_field (di, "strg", IDENTIFIER_POINTER (t)); > dump_int (di, "lngt", IDENTIFIER_LENGTH (t)); > @@ -435,6 +433,7 @@ dequeue_and_dump (dump_info_p di) > > case STATEMENT_LIST: > { > + int i; > tree_stmt_iterator it; > for (i = 0, it = tsi_start (t); !tsi_end_p (it); tsi_next (&it), i++) > { > @@ -447,7 +446,7 @@ dequeue_and_dump (dump_info_p di) > > case TREE_VEC: > dump_int (di, "lngt", TREE_VEC_LENGTH (t)); > - for (i = 0; i < TREE_VEC_LENGTH (t); ++i) > + for (int i = 0; i < TREE_VEC_LENGTH (t); ++i) > { > char buffer[32]; > sprintf (buffer, "%u", i); > @@ -707,9 +706,8 @@ dequeue_and_dump (dump_info_p di) > break; > case OMP_CLAUSE: > { > - int i; > fprintf (di->stream, "%s\n", omp_clause_code_name[OMP_CLAUSE_CODE (t)]); > - for (i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) > + for (int i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) > dump_child ("op: ", OMP_CLAUSE_OPERAND (t, i)); > } > break; > -- > 2.10.1 > Jakub
On 11/01/2016 03:53 PM, Jakub Jelinek wrote: > On Tue, Nov 01, 2016 at 03:47:54PM +0100, Martin Liška wrote: >> On 10/27/2016 07:23 PM, Jakub Jelinek wrote: >>> Ok for trunk with that change. >>> >>> Jakub >> >> Hello. >> >> I'll commit the patch as soon as following patch would be accepted. The patch >> fixes false positives when running asan-bootstrap. > > What kind of false positives it is for each case? Is it with normal > asan-bootstrap (without your -fsanitize-use-after-scope changes), or > only with those changes, or only with those changes and > -fsanitize-use-after-scope used during bootstrap? It's only with those changes as -fsanitize-address-use-after-scope is enabled by default with -fsanitize=address. Current bootstrap-asan works fine. I'll re-trigger the bootstrap again, but IIRC the main culprit was ASAN_MARK poisoning done at switch labels (which should be partially fixed with a newer version of the patch). Martin > >> >From b62e4d7ffe659ec338ef83e84ccb572a07264283 Mon Sep 17 00:00:00 2001 >> From: marxin <mliska@suse.cz> >> Date: Tue, 20 Sep 2016 10:31:25 +0200 >> Subject: [PATCH 1/4] Fix ASAN bootstrap uninitialized warning. >> >> gcc/ChangeLog: >> >> 2016-09-26 Martin Liska <mliska@suse.cz> >> >> * ipa-devirt.c (record_targets_from_bases): Initialize a >> variable. >> * omp-low.c (lower_omp_target): Remove a variable from >> scope defined by a switch statement. >> * tree-dump.c (dequeue_and_dump): Likewise. >> >> gcc/java/ChangeLog: >> >> 2016-09-26 Martin Liska <mliska@suse.cz> >> >> * mangle.c (mangle_type): Remove a variable from >> scope defined by a switch statement. >> --- >> gcc/ipa-devirt.c | 2 +- >> gcc/omp-low.c | 11 ++++------- >> gcc/tree-dump.c | 8 +++----- >> 3 files changed, 8 insertions(+), 13 deletions(-) >> >> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c >> index 49e2195..5c0ae72 100644 >> --- a/gcc/ipa-devirt.c >> +++ b/gcc/ipa-devirt.c >> @@ -2862,7 +2862,7 @@ record_targets_from_bases (tree otr_type, >> { >> while (true) >> { >> - HOST_WIDE_INT pos, size; >> + HOST_WIDE_INT pos = 0, size; >> tree base_binfo; >> tree fld; >> >> diff --git a/gcc/omp-low.c b/gcc/omp-low.c >> index e5b9e4c..62c9e5c 100644 >> --- a/gcc/omp-low.c >> +++ b/gcc/omp-low.c >> @@ -15803,11 +15803,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) >> push_gimplify_context (); >> fplist = NULL; >> >> + tree var, x; >> for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) >> switch (OMP_CLAUSE_CODE (c)) >> { >> - tree var, x; >> - >> default: >> break; >> case OMP_CLAUSE_MAP: >> @@ -16066,12 +16065,11 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) >> vec_alloc (vkind, map_cnt); >> unsigned int map_idx = 0; >> >> + tree ovar, nc, s, purpose, var, x, type; >> + unsigned int talign; >> for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) >> switch (OMP_CLAUSE_CODE (c)) >> { >> - tree ovar, nc, s, purpose, var, x, type; >> - unsigned int talign; >> - >> default: >> break; >> >> @@ -16442,10 +16440,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) >> if (offloaded || data_region) >> { >> tree prev = NULL_TREE; >> + tree var, x; >> for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) >> switch (OMP_CLAUSE_CODE (c)) >> { >> - tree var, x; >> default: >> break; >> case OMP_CLAUSE_FIRSTPRIVATE: >> @@ -16594,7 +16592,6 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) >> for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) >> switch (OMP_CLAUSE_CODE (c)) >> { >> - tree var; >> default: >> break; >> case OMP_CLAUSE_MAP: >> diff --git a/gcc/tree-dump.c b/gcc/tree-dump.c >> index df47181..89f72a0 100644 >> --- a/gcc/tree-dump.c >> +++ b/gcc/tree-dump.c >> @@ -420,8 +420,6 @@ dequeue_and_dump (dump_info_p di) >> /* Now handle the various kinds of nodes. */ >> switch (code) >> { >> - int i; >> - >> case IDENTIFIER_NODE: >> dump_string_field (di, "strg", IDENTIFIER_POINTER (t)); >> dump_int (di, "lngt", IDENTIFIER_LENGTH (t)); >> @@ -435,6 +433,7 @@ dequeue_and_dump (dump_info_p di) >> >> case STATEMENT_LIST: >> { >> + int i; >> tree_stmt_iterator it; >> for (i = 0, it = tsi_start (t); !tsi_end_p (it); tsi_next (&it), i++) >> { >> @@ -447,7 +446,7 @@ dequeue_and_dump (dump_info_p di) >> >> case TREE_VEC: >> dump_int (di, "lngt", TREE_VEC_LENGTH (t)); >> - for (i = 0; i < TREE_VEC_LENGTH (t); ++i) >> + for (int i = 0; i < TREE_VEC_LENGTH (t); ++i) >> { >> char buffer[32]; >> sprintf (buffer, "%u", i); >> @@ -707,9 +706,8 @@ dequeue_and_dump (dump_info_p di) >> break; >> case OMP_CLAUSE: >> { >> - int i; >> fprintf (di->stream, "%s\n", omp_clause_code_name[OMP_CLAUSE_CODE (t)]); >> - for (i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) >> + for (int i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) >> dump_child ("op: ", OMP_CLAUSE_OPERAND (t, i)); >> } >> break; >> -- >> 2.10.1 >> > > > Jakub >
On 11/01/2016 03:53 PM, Jakub Jelinek wrote: > What kind of false positives it is for each case? Is it with normal > asan-bootstrap (without your -fsanitize-use-after-scope changes), or > only with those changes, or only with those changes and > -fsanitize-use-after-scope used during bootstrap? Ok, the situation is simpler than I thought: #include <stdio.h> int main(int argc, char **argv) { int *ptr; switch (argc) { int a; case 1: break; default: ptr = &a; break; } fprintf (stderr, "v: %d\n", *ptr); return 0; } Which is gimplified as: int * ptr; switch (argc) <default: <D.2575>, case 1: <D.2573>> { int a; try { ASAN_MARK (2, &a, 4); <D.2573>: goto <D.2574>; <D.2575>: ptr = &a; goto <D.2574>; } finally { ASAN_MARK (1, &a, 4); } } <D.2574>: _1 = *ptr; stderr.0_2 = stderr; fprintf (stderr.0_2, "v: %d\n", _1); D.2577 = 0; return D.2577; } D.2577 = 0; return D.2577; and thus we get: /tmp/switch-case.c:9:11: warning: statement will never be executed [-Wswitch-unreachable] int a; I'm wondering where properly fix that, we can either find all these ASAN_MARKs in gimplify_switch_expr and distribute it to all labels (which are gimplified). Or we can put such variables to asan_poisoned_variables if we have information that we're gimplifing statements before a first label. Do we know that from gimple context? If so, these variables will be unpoisoned at the very beginning of each label and the ASAN_MARK call in between switch statement and a first label can be removed. Thoughts? Thanks, Martin
On Wed, Nov 02, 2016 at 10:36:44AM +0100, Martin Liška wrote: > On 11/01/2016 03:53 PM, Jakub Jelinek wrote: > > What kind of false positives it is for each case? Is it with normal > > asan-bootstrap (without your -fsanitize-use-after-scope changes), or > > only with those changes, or only with those changes and > > -fsanitize-use-after-scope used during bootstrap? > > Ok, the situation is simpler than I thought: CCing also Marek. > > #include <stdio.h> > > int main(int argc, char **argv) > { > int *ptr; > > switch (argc) > { > int a; > > case 1: > break; > > default: > ptr = &a; > break; > } > > fprintf (stderr, "v: %d\n", *ptr); > return 0; > } > > Which is gimplified as: > > int * ptr; > > switch (argc) <default: <D.2575>, case 1: <D.2573>> > { > int a; > > try > { > ASAN_MARK (2, &a, 4); > <D.2573>: > goto <D.2574>; > <D.2575>: > ptr = &a; > goto <D.2574>; > } > finally > { > ASAN_MARK (1, &a, 4); > } > } > <D.2574>: > _1 = *ptr; > stderr.0_2 = stderr; > fprintf (stderr.0_2, "v: %d\n", _1); > D.2577 = 0; > return D.2577; > } > D.2577 = 0; > return D.2577; > > and thus we get: > /tmp/switch-case.c:9:11: warning: statement will never be executed [-Wswitch-unreachable] > int a; > > I'm wondering where properly fix that, we can either find all these ASAN_MARKs in gimplify_switch_expr > and distribute it to all labels (which are gimplified). Or we can put such variables to asan_poisoned_variables > if we have information that we're gimplifing statements before a first label. Do we know that from gimple context? > If so, these variables will be unpoisoned at the very beginning of each label and the ASAN_MARK call in between > switch statement and a first label can be removed. Wouldn't it be easiest if -Wswitch-unreachable warning just ignored the ASAN_MARK internal calls altogether? Do you emit there any other statements, or just ASAN_MARK and nothing else? Shouldn't there be also ASAN_MARK on the implicit gotos from the switch statement? Otherwise, consider this being done in a loop, after the first iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with args 1 and have in case 1: a = 0;, won't that trigger runtime error? Jakub
On 11/02/2016 10:59 AM, Jakub Jelinek wrote: > On Wed, Nov 02, 2016 at 10:36:44AM +0100, Martin Liška wrote: >> On 11/01/2016 03:53 PM, Jakub Jelinek wrote: >>> What kind of false positives it is for each case? Is it with normal >>> asan-bootstrap (without your -fsanitize-use-after-scope changes), or >>> only with those changes, or only with those changes and >>> -fsanitize-use-after-scope used during bootstrap? >> >> Ok, the situation is simpler than I thought: > > CCing also Marek. >> >> #include <stdio.h> >> >> int main(int argc, char **argv) >> { >> int *ptr; >> >> switch (argc) >> { >> int a; >> >> case 1: >> break; >> >> default: >> ptr = &a; >> break; >> } >> >> fprintf (stderr, "v: %d\n", *ptr); >> return 0; >> } >> >> Which is gimplified as: >> >> int * ptr; >> >> switch (argc) <default: <D.2575>, case 1: <D.2573>> >> { >> int a; >> >> try >> { >> ASAN_MARK (2, &a, 4); >> <D.2573>: >> goto <D.2574>; >> <D.2575>: >> ptr = &a; >> goto <D.2574>; >> } >> finally >> { >> ASAN_MARK (1, &a, 4); >> } >> } >> <D.2574>: >> _1 = *ptr; >> stderr.0_2 = stderr; >> fprintf (stderr.0_2, "v: %d\n", _1); >> D.2577 = 0; >> return D.2577; >> } >> D.2577 = 0; >> return D.2577; >> >> and thus we get: >> /tmp/switch-case.c:9:11: warning: statement will never be executed [-Wswitch-unreachable] >> int a; >> >> I'm wondering where properly fix that, we can either find all these ASAN_MARKs in gimplify_switch_expr >> and distribute it to all labels (which are gimplified). Or we can put such variables to asan_poisoned_variables >> if we have information that we're gimplifing statements before a first label. Do we know that from gimple context? >> If so, these variables will be unpoisoned at the very beginning of each label and the ASAN_MARK call in between >> switch statement and a first label can be removed. > > Wouldn't it be easiest if -Wswitch-unreachable warning just ignored > the ASAN_MARK internal calls altogether? > Do you emit there any other statements, or just ASAN_MARK and nothing else? Yep, skipping warning can be done easily, however gimplified code is wrong as un-poisoning is not done for variable (even for a valid program). > > Shouldn't there be also ASAN_MARK on the implicit gotos from the switch > statement? Otherwise, consider this being done in a loop, after the first > iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with > args 1 and have in case 1: a = 0;, won't that trigger runtime error? Hopefully having the un-poisoning call be encapsulated in finally block would properly clean up the variable. Or am I wrong? Martin > > Jakub >
On Wed, Nov 02, 2016 at 10:59:26AM +0100, Jakub Jelinek wrote: > > Which is gimplified as: > > > > int * ptr; > > > > switch (argc) <default: <D.2575>, case 1: <D.2573>> > > { > > int a; > > > > try > > { > > ASAN_MARK (2, &a, 4); > > <D.2573>: > > goto <D.2574>; > > <D.2575>: > > ptr = &a; > > goto <D.2574>; > > } > > finally > > { > > ASAN_MARK (1, &a, 4); > > } > Shouldn't there be also ASAN_MARK on the implicit gotos from the switch > statement? Otherwise, consider this being done in a loop, after the first > iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with > args 1 and have in case 1: a = 0;, won't that trigger runtime error? Wonder if for the variables declared inside of switch body, because we don't care about uses before scope, it wouldn't be more efficient to arrange for int *p, *q, *r; switch (x) { int a; case 1: p = &a; a = 5; break; int b; case 2: int c; q = &b; r = &c; b = 3; c = 4; break; } to effectively ASAN_MARK (2, &a, 4); ASAN_MARK (2, &b, 4); ASAN_MARK (2, &c, 4); before the GIMPLE_SWITCH, instead of unpoisoning them on every case label where they might be in scope. Though, of course, at least until lower pass that is quite ugly, because it would refer to "not yet declared" variables. Perhaps we'd need to move the ASAN_MARK and GIMPLE_SWITCH (but of course not the expression evaluation of the switch control expression) inside of the switches' GIMPLE_BIND. Jakub
On Wed, Nov 02, 2016 at 11:10:53AM +0100, Jakub Jelinek wrote: > On Wed, Nov 02, 2016 at 10:59:26AM +0100, Jakub Jelinek wrote: > > > Which is gimplified as: > > > > > > int * ptr; > > > > > > switch (argc) <default: <D.2575>, case 1: <D.2573>> > > > { > > > int a; > > > > > > try > > > { > > > ASAN_MARK (2, &a, 4); > > > <D.2573>: > > > goto <D.2574>; > > > <D.2575>: > > > ptr = &a; > > > goto <D.2574>; > > > } > > > finally > > > { > > > ASAN_MARK (1, &a, 4); > > > } > > > Shouldn't there be also ASAN_MARK on the implicit gotos from the switch > > statement? Otherwise, consider this being done in a loop, after the first > > iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with > > args 1 and have in case 1: a = 0;, won't that trigger runtime error? > > Wonder if for the variables declared inside of switch body, because we don't > care about uses before scope, it wouldn't be more efficient to arrange for > int *p, *q, *r; > switch (x) > { > int a; > case 1: > p = &a; > a = 5; > break; > int b; > case 2: > int c; > q = &b; > r = &c; > b = 3; > c = 4; > break; > } > to effectively ASAN_MARK (2, &a, 4); ASAN_MARK (2, &b, 4); ASAN_MARK (2, &c, 4); > before the GIMPLE_SWITCH, instead of unpoisoning them on every case label > where they might be in scope. Though, of course, at least until lower pass > that is quite ugly, because it would refer to "not yet declared" variables. > Perhaps we'd need to move the ASAN_MARK and GIMPLE_SWITCH (but of course not > the expression evaluation of the switch control expression) inside of the > switches' GIMPLE_BIND. So is there anything I should do wrt -Wswitch-unreachable? Marek
On 11/02/2016 03:20 PM, Marek Polacek wrote: > On Wed, Nov 02, 2016 at 11:10:53AM +0100, Jakub Jelinek wrote: >> On Wed, Nov 02, 2016 at 10:59:26AM +0100, Jakub Jelinek wrote: >>>> Which is gimplified as: >>>> >>>> int * ptr; >>>> >>>> switch (argc) <default: <D.2575>, case 1: <D.2573>> >>>> { >>>> int a; >>>> >>>> try >>>> { >>>> ASAN_MARK (2, &a, 4); >>>> <D.2573>: >>>> goto <D.2574>; >>>> <D.2575>: >>>> ptr = &a; >>>> goto <D.2574>; >>>> } >>>> finally >>>> { >>>> ASAN_MARK (1, &a, 4); >>>> } >> >>> Shouldn't there be also ASAN_MARK on the implicit gotos from the switch >>> statement? Otherwise, consider this being done in a loop, after the first >>> iteration you ASAN_MARK (1, &a, 4) (i.e. poison), then you iterate say with >>> args 1 and have in case 1: a = 0;, won't that trigger runtime error? >> >> Wonder if for the variables declared inside of switch body, because we don't >> care about uses before scope, it wouldn't be more efficient to arrange for >> int *p, *q, *r; >> switch (x) >> { >> int a; >> case 1: >> p = &a; >> a = 5; >> break; >> int b; >> case 2: >> int c; >> q = &b; >> r = &c; >> b = 3; >> c = 4; >> break; >> } >> to effectively ASAN_MARK (2, &a, 4); ASAN_MARK (2, &b, 4); ASAN_MARK (2, &c, 4); >> before the GIMPLE_SWITCH, instead of unpoisoning them on every case label >> where they might be in scope. Though, of course, at least until lower pass >> that is quite ugly, because it would refer to "not yet declared" variables. >> Perhaps we'd need to move the ASAN_MARK and GIMPLE_SWITCH (but of course not >> the expression evaluation of the switch control expression) inside of the >> switches' GIMPLE_BIND. > > So is there anything I should do wrt -Wswitch-unreachable? > > Marek > Probably not. I'm having a patch puts GIMPLE_SWITCH statement to a proper place in GIMPLE_BIND. Let's see whether such patch can bootstrap and survive regression tests. Thanks, Martin
On Wed, Nov 02, 2016 at 03:27:42PM +0100, Martin Liška wrote: > > So is there anything I should do wrt -Wswitch-unreachable? > > > > Marek > > > > Probably not. I'm having a patch puts GIMPLE_SWITCH statement to a proper place > in GIMPLE_BIND. Let's see whether such patch can bootstrap and survive regression > tests. Please do that only for -fsanitize-use-after-scope, it will likely affect at least for -O0 the debugging experience. For -O0 -fsanitize=address -fsanitize-use-after-scope perhaps we could arrange for some extra stmt to have the locus of the switch (where we still don't want the vars to appear in scope) and then have no locus on the ASAN_MARK and actual GIMPLE_SWITCH or something similar. Jakub
From b62e4d7ffe659ec338ef83e84ccb572a07264283 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 20 Sep 2016 10:31:25 +0200 Subject: [PATCH 1/4] Fix ASAN bootstrap uninitialized warning. gcc/ChangeLog: 2016-09-26 Martin Liska <mliska@suse.cz> * ipa-devirt.c (record_targets_from_bases): Initialize a variable. * omp-low.c (lower_omp_target): Remove a variable from scope defined by a switch statement. * tree-dump.c (dequeue_and_dump): Likewise. gcc/java/ChangeLog: 2016-09-26 Martin Liska <mliska@suse.cz> * mangle.c (mangle_type): Remove a variable from scope defined by a switch statement. --- gcc/ipa-devirt.c | 2 +- gcc/omp-low.c | 11 ++++------- gcc/tree-dump.c | 8 +++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c index 49e2195..5c0ae72 100644 --- a/gcc/ipa-devirt.c +++ b/gcc/ipa-devirt.c @@ -2862,7 +2862,7 @@ record_targets_from_bases (tree otr_type, { while (true) { - HOST_WIDE_INT pos, size; + HOST_WIDE_INT pos = 0, size; tree base_binfo; tree fld; diff --git a/gcc/omp-low.c b/gcc/omp-low.c index e5b9e4c..62c9e5c 100644 --- a/gcc/omp-low.c +++ b/gcc/omp-low.c @@ -15803,11 +15803,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) push_gimplify_context (); fplist = NULL; + tree var, x; for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) switch (OMP_CLAUSE_CODE (c)) { - tree var, x; - default: break; case OMP_CLAUSE_MAP: @@ -16066,12 +16065,11 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) vec_alloc (vkind, map_cnt); unsigned int map_idx = 0; + tree ovar, nc, s, purpose, var, x, type; + unsigned int talign; for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) switch (OMP_CLAUSE_CODE (c)) { - tree ovar, nc, s, purpose, var, x, type; - unsigned int talign; - default: break; @@ -16442,10 +16440,10 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) if (offloaded || data_region) { tree prev = NULL_TREE; + tree var, x; for (c = clauses; c ; c = OMP_CLAUSE_CHAIN (c)) switch (OMP_CLAUSE_CODE (c)) { - tree var, x; default: break; case OMP_CLAUSE_FIRSTPRIVATE: @@ -16594,7 +16592,6 @@ lower_omp_target (gimple_stmt_iterator *gsi_p, omp_context *ctx) for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c)) switch (OMP_CLAUSE_CODE (c)) { - tree var; default: break; case OMP_CLAUSE_MAP: diff --git a/gcc/tree-dump.c b/gcc/tree-dump.c index df47181..89f72a0 100644 --- a/gcc/tree-dump.c +++ b/gcc/tree-dump.c @@ -420,8 +420,6 @@ dequeue_and_dump (dump_info_p di) /* Now handle the various kinds of nodes. */ switch (code) { - int i; - case IDENTIFIER_NODE: dump_string_field (di, "strg", IDENTIFIER_POINTER (t)); dump_int (di, "lngt", IDENTIFIER_LENGTH (t)); @@ -435,6 +433,7 @@ dequeue_and_dump (dump_info_p di) case STATEMENT_LIST: { + int i; tree_stmt_iterator it; for (i = 0, it = tsi_start (t); !tsi_end_p (it); tsi_next (&it), i++) { @@ -447,7 +446,7 @@ dequeue_and_dump (dump_info_p di) case TREE_VEC: dump_int (di, "lngt", TREE_VEC_LENGTH (t)); - for (i = 0; i < TREE_VEC_LENGTH (t); ++i) + for (int i = 0; i < TREE_VEC_LENGTH (t); ++i) { char buffer[32]; sprintf (buffer, "%u", i); @@ -707,9 +706,8 @@ dequeue_and_dump (dump_info_p di) break; case OMP_CLAUSE: { - int i; fprintf (di->stream, "%s\n", omp_clause_code_name[OMP_CLAUSE_CODE (t)]); - for (i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) + for (int i = 0; i < omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; i++) dump_child ("op: ", OMP_CLAUSE_OPERAND (t, i)); } break; -- 2.10.1