* [PATCH] analyzer: deal with -fshort-enums @ 2023-11-19 7:39 Alexandre Oliva 2023-11-20 22:32 ` David Malcolm 0 siblings, 1 reply; 11+ messages in thread From: Alexandre Oliva @ 2023-11-19 7:39 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm On platforms that enable -fshort-enums by default, various switch-enum analyzer tests fail, because apply_constraints_for_gswitch doesn't expect the integral promotion type cast. I've arranged for the code to cope with those casts. Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default cpu on trunk, and with tms570 on gcc-13. Ok to install? for gcc/analyzer/ChangeLog * region-model.cc (has_nondefault_case_for_value_p): Take enumerate type as a parameter. (region_model::apply_constraints_for_gswitch): Cope with integral promotion type casts. --- gcc/analyzer/region-model.cc | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index dc83440652027..05f716cb7d663 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -5340,10 +5340,10 @@ has_nondefault_case_for_value_p (const gswitch *switch_stmt, tree int_cst) has nondefault cases handling all values in the enum. */ static bool -has_nondefault_cases_for_all_enum_values_p (const gswitch *switch_stmt) +has_nondefault_cases_for_all_enum_values_p (const gswitch *switch_stmt, + tree type) { gcc_assert (switch_stmt); - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); for (tree enum_val_iter = TYPE_VALUES (type); @@ -5379,6 +5379,23 @@ apply_constraints_for_gswitch (const switch_cfg_superedge &edge, { tree index = gimple_switch_index (switch_stmt); const svalue *index_sval = get_rvalue (index, ctxt); + bool check_index_type = true; + + /* With -fshort-enum, there may be a type cast. */ + if (ctxt && index_sval->get_kind () == SK_UNARYOP + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) + { + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> (index_sval); + if (unaryop->get_op () == NOP_EXPR + && is_a <const initial_svalue *> (unaryop->get_arg ())) + if (const initial_svalue *initvalop = (as_a <const initial_svalue *> + (unaryop->get_arg ()))) + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) + { + index_sval = initvalop; + check_index_type = false; + } + } /* If we're switching based on an enum type, assume that the user is only working with values from the enum. Hence if this is an @@ -5390,12 +5407,14 @@ apply_constraints_for_gswitch (const switch_cfg_superedge &edge, ctxt /* Must be an enum value. */ && index_sval->get_type () - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE + && (!check_index_type + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE /* If we have a constant, then we can check it directly. */ && index_sval->get_kind () != SK_CONSTANT && edge.implicitly_created_default_p () - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, + index_sval->get_type ()) /* Don't do this if there's a chance that the index is attacker-controlled. */ && !ctxt->possibly_tainted_p (index_sval)) -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] analyzer: deal with -fshort-enums 2023-11-19 7:39 [PATCH] analyzer: deal with -fshort-enums Alexandre Oliva @ 2023-11-20 22:32 ` David Malcolm 2023-11-22 7:58 ` Alexandre Oliva 0 siblings, 1 reply; 11+ messages in thread From: David Malcolm @ 2023-11-20 22:32 UTC (permalink / raw) To: Alexandre Oliva, gcc-patches On Sun, 2023-11-19 at 04:39 -0300, Alexandre Oliva wrote: > > On platforms that enable -fshort-enums by default, various switch- > enum > analyzer tests fail, because apply_constraints_for_gswitch doesn't > expect the integral promotion type cast. I've arranged for the code > to cope with those casts. Thanks for the patch, and sorry for the failing tests. Which tests failed? I'm guessing one of the ones from r13-5159- gccd4df81aa6537. Can you add a copy of one of those tests that needs the patch, with an explicit -fshort-enums, to ensure that regression testing is hitting this case on all targets in the future? Thanks Dave > > Regstrapped on x86_64-linux-gnu, also tested on arm-eabi with default > cpu on trunk, and with tms570 on gcc-13. Ok to install? > > > for gcc/analyzer/ChangeLog > > * region-model.cc (has_nondefault_case_for_value_p): Take > enumerate type as a parameter. > (region_model::apply_constraints_for_gswitch): Cope with > integral promotion type casts. > --- > gcc/analyzer/region-model.cc | 27 +++++++++++++++++++++++---- > 1 file changed, 23 insertions(+), 4 deletions(-) > > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- > model.cc > index dc83440652027..05f716cb7d663 100644 > --- a/gcc/analyzer/region-model.cc > +++ b/gcc/analyzer/region-model.cc > @@ -5340,10 +5340,10 @@ has_nondefault_case_for_value_p (const > gswitch *switch_stmt, tree int_cst) > has nondefault cases handling all values in the enum. */ > > static bool > -has_nondefault_cases_for_all_enum_values_p (const gswitch > *switch_stmt) > +has_nondefault_cases_for_all_enum_values_p (const gswitch > *switch_stmt, > + tree type) > { > gcc_assert (switch_stmt); > - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); > gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); > > for (tree enum_val_iter = TYPE_VALUES (type); > @@ -5379,6 +5379,23 @@ apply_constraints_for_gswitch (const > switch_cfg_superedge &edge, > { > tree index = gimple_switch_index (switch_stmt); > const svalue *index_sval = get_rvalue (index, ctxt); > + bool check_index_type = true; > + > + /* With -fshort-enum, there may be a type cast. */ > + if (ctxt && index_sval->get_kind () == SK_UNARYOP > + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) > + { > + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> > (index_sval); > + if (unaryop->get_op () == NOP_EXPR > + && is_a <const initial_svalue *> (unaryop->get_arg ())) > + if (const initial_svalue *initvalop = (as_a <const > initial_svalue *> > + (unaryop->get_arg > ()))) > + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) > + { > + index_sval = initvalop; > + check_index_type = false; > + } > + } > > /* If we're switching based on an enum type, assume that the user > is only > working with values from the enum. Hence if this is an > @@ -5390,12 +5407,14 @@ apply_constraints_for_gswitch (const > switch_cfg_superedge &edge, > ctxt > /* Must be an enum value. */ > && index_sval->get_type () > - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE > + && (!check_index_type > + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) > && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE > /* If we have a constant, then we can check it directly. */ > && index_sval->get_kind () != SK_CONSTANT > && edge.implicitly_created_default_p () > - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) > + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, > + index_sval- > >get_type ()) > /* Don't do this if there's a chance that the index is > attacker-controlled. */ > && !ctxt->possibly_tainted_p (index_sval)) > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] analyzer: deal with -fshort-enums 2023-11-20 22:32 ` David Malcolm @ 2023-11-22 7:58 ` Alexandre Oliva 2023-12-06 5:31 ` Alexandre Oliva 0 siblings, 1 reply; 11+ messages in thread From: Alexandre Oliva @ 2023-11-22 7:58 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches On Nov 20, 2023, David Malcolm <dmalcolm@redhat.com> wrote: > On Sun, 2023-11-19 at 04:39 -0300, Alexandre Oliva wrote: >> >> On platforms that enable -fshort-enums by default, various switch- >> enum >> analyzer tests fail, because apply_constraints_for_gswitch doesn't >> expect the integral promotion type cast. I've arranged for the code >> to cope with those casts. > Thanks for the patch, and sorry for the failing tests. No worries. > Which tests failed? I'm guessing one of the ones from r13-5159- > gccd4df81aa6537. Yeah, I'm afraid I didn't take notes about which tests failed (I'll try to dig that up), but I'm sure switch-enum-1, switch-enum-2, and switch-enum-pr105273-doom-p_maputl.c. I *think* also switch-enum-5 and switch-enum-pr105273-doom-p_floor.c. > Can you add a copy of one of those tests that needs the patch, with an > explicit -fshort-enums, to ensure that regression testing is hitting > this case on all targets in the future? Ah, nice, that's a great idea, I wish I'd thought of that! Will do. -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] analyzer: deal with -fshort-enums 2023-11-22 7:58 ` Alexandre Oliva @ 2023-12-06 5:31 ` Alexandre Oliva 2023-12-06 22:22 ` David Malcolm 0 siblings, 1 reply; 11+ messages in thread From: Alexandre Oliva @ 2023-12-06 5:31 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: > Ah, nice, that's a great idea, I wish I'd thought of that! Will do. Sorry it took me so long, here it is. I added two tests, so that, regardless of the defaults, we get both circumstances tested, without repetition. Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to install? analyzer: deal with -fshort-enums On platforms that enable -fshort-enums by default, various switch-enum analyzer tests fail, because apply_constraints_for_gswitch doesn't expect the integral promotion type cast. I've arranged for the code to cope with those casts. for gcc/analyzer/ChangeLog * region-model.cc (has_nondefault_case_for_value_p): Take enumerate type as a parameter. (region_model::apply_constraints_for_gswitch): Cope with integral promotion type casts. for gcc/testsuite/ChangeLog * gcc.dg/analyzer/switch-short-enum-1.c: New. * gcc.dg/analyzer/switch-no-short-enum-1.c: New. --- gcc/analyzer/region-model.cc | 27 +++- .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 ++++++++++++++++++++ .../gcc.dg/analyzer/switch-short-enum-1.c | 140 ++++++++++++++++++++ 3 files changed, 304 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 2157ad2578b85..6a7a8bc9f4884 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const gswitch *switch_stmt, tree int_cst) has nondefault cases handling all values in the enum. */ static bool -has_nondefault_cases_for_all_enum_values_p (const gswitch *switch_stmt) +has_nondefault_cases_for_all_enum_values_p (const gswitch *switch_stmt, + tree type) { gcc_assert (switch_stmt); - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); for (tree enum_val_iter = TYPE_VALUES (type); @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const switch_cfg_superedge &edge, { tree index = gimple_switch_index (switch_stmt); const svalue *index_sval = get_rvalue (index, ctxt); + bool check_index_type = true; + + /* With -fshort-enum, there may be a type cast. */ + if (ctxt && index_sval->get_kind () == SK_UNARYOP + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) + { + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> (index_sval); + if (unaryop->get_op () == NOP_EXPR + && is_a <const initial_svalue *> (unaryop->get_arg ())) + if (const initial_svalue *initvalop = (as_a <const initial_svalue *> + (unaryop->get_arg ()))) + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) + { + index_sval = initvalop; + check_index_type = false; + } + } /* If we're switching based on an enum type, assume that the user is only working with values from the enum. Hence if this is an @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const switch_cfg_superedge &edge, ctxt /* Must be an enum value. */ && index_sval->get_type () - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE + && (!check_index_type + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE /* If we have a constant, then we can check it directly. */ && index_sval->get_kind () != SK_CONSTANT && edge.implicitly_created_default_p () - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, + index_sval->get_type ()) /* Don't do this if there's a chance that the index is attacker-controlled. */ && !ctxt->possibly_tainted_p (index_sval)) diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c new file mode 100644 index 0000000000000..98f6d91f97481 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c @@ -0,0 +1,141 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-fno-short-enums" } */ +/* { dg-skip-if "default" { ! short_enums } } */ + +#include "analyzer-decls.h" + +/* Verify the handling of "switch (enum_value)". */ + +enum e +{ + E_VAL0, + E_VAL1, + E_VAL2 +}; + +/* Verify that we assume that "switch (enum)" doesn't follow implicit + "default" if all enum values have cases */ + +int test_all_values_covered_implicit_default_1 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + case E_VAL2: + return 1945; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ +} + +int test_all_values_covered_implicit_default_2 (enum e x) +{ + int result; + switch (x) + { + case E_VAL0: + result = 1066; + break; + case E_VAL1: + result = 1776; + break; + case E_VAL2: + result = 1945; + break; + } + return result; /* { dg-bogus "uninitialized" } */ +} + +/* Verify that we consider paths that use the implicit default when not + all enum values are covered by cases. */ + +int test_missing_values_implicit_default_1 (enum e x) +{ + switch (x) /* { dg-message "following 'default:' branch" } */ + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + } + __analyzer_dump_path (); /* { dg-message "path" } */ + return 0; +} + +int test_missing_values_implicit_default_2 (enum e x) +{ + int result; + switch (x) /* { dg-message "following 'default:' branch" } */ + { + case E_VAL0: + result = 1066; + break; + case E_VAL1: + result = 1776; + break; + } + return result; /* { dg-warning "uninitialized" } */ +} + +/* Verify that explicit "default" isn't rejected. */ + +int test_all_values_covered_explicit_default_1 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + case E_VAL2: + return 1945; + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 0; + } +} + +int test_missing_values_explicit_default_1 (enum e x) +{ + switch (x) + { + default: + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} + +int test_missing_values_explicit_default_2 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 1945; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} + +int test_just_default (enum e x) +{ + switch (x) + { + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 42; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} + diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c new file mode 100644 index 0000000000000..384113fde5cbf --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c @@ -0,0 +1,140 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-fshort-enums" } */ +/* { dg-skip-if "default" { short_enums } } */ + +#include "analyzer-decls.h" + +/* Verify the handling of "switch (enum_value)". */ + +enum e +{ + E_VAL0, + E_VAL1, + E_VAL2 +}; + +/* Verify that we assume that "switch (enum)" doesn't follow implicit + "default" if all enum values have cases */ + +int test_all_values_covered_implicit_default_1 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + case E_VAL2: + return 1945; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ +} + +int test_all_values_covered_implicit_default_2 (enum e x) +{ + int result; + switch (x) + { + case E_VAL0: + result = 1066; + break; + case E_VAL1: + result = 1776; + break; + case E_VAL2: + result = 1945; + break; + } + return result; /* { dg-bogus "uninitialized" } */ +} + +/* Verify that we consider paths that use the implicit default when not + all enum values are covered by cases. */ + +int test_missing_values_implicit_default_1 (enum e x) +{ + switch (x) /* { dg-message "following 'default:' branch" } */ + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + } + __analyzer_dump_path (); /* { dg-message "path" } */ + return 0; +} + +int test_missing_values_implicit_default_2 (enum e x) +{ + int result; + switch (x) /* { dg-message "following 'default:' branch" } */ + { + case E_VAL0: + result = 1066; + break; + case E_VAL1: + result = 1776; + break; + } + return result; /* { dg-warning "uninitialized" } */ +} + +/* Verify that explicit "default" isn't rejected. */ + +int test_all_values_covered_explicit_default_1 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + case E_VAL2: + return 1945; + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 0; + } +} + +int test_missing_values_explicit_default_1 (enum e x) +{ + switch (x) + { + default: + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} + +int test_missing_values_explicit_default_2 (enum e x) +{ + switch (x) + { + case E_VAL0: + return 1066; + case E_VAL1: + return 1776; + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 1945; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} + +int test_just_default (enum e x) +{ + switch (x) + { + default: + __analyzer_dump_path (); /* { dg-message "path" } */ + return 42; + } + __analyzer_dump_path (); /* { dg-bogus "path" } */ + return 0; +} -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] analyzer: deal with -fshort-enums 2023-12-06 5:31 ` Alexandre Oliva @ 2023-12-06 22:22 ` David Malcolm 2024-02-07 16:21 ` Torbjorn SVENSSON 0 siblings, 1 reply; 11+ messages in thread From: David Malcolm @ 2023-12-06 22:22 UTC (permalink / raw) To: Alexandre Oliva; +Cc: gcc-patches On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: > On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: > > > Ah, nice, that's a great idea, I wish I'd thought of that! Will > > do. > > Sorry it took me so long, here it is. I added two tests, so that, > regardless of the defaults, we get both circumstances tested, without > repetition. > > Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to > install? Thanks for the updated patch. Looks good to me. Dave > > > analyzer: deal with -fshort-enums > > On platforms that enable -fshort-enums by default, various switch- > enum > analyzer tests fail, because apply_constraints_for_gswitch doesn't > expect the integral promotion type cast. I've arranged for the code > to cope with those casts. > > > for gcc/analyzer/ChangeLog > > * region-model.cc (has_nondefault_case_for_value_p): Take > enumerate type as a parameter. > (region_model::apply_constraints_for_gswitch): Cope with > integral promotion type casts. > > for gcc/testsuite/ChangeLog > > * gcc.dg/analyzer/switch-short-enum-1.c: New. > * gcc.dg/analyzer/switch-no-short-enum-1.c: New. > --- > gcc/analyzer/region-model.cc | 27 +++- > .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 > ++++++++++++++++++++ > .../gcc.dg/analyzer/switch-short-enum-1.c | 140 > ++++++++++++++++++++ > 3 files changed, 304 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- > enum-1.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- > 1.c > > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- > model.cc > index 2157ad2578b85..6a7a8bc9f4884 100644 > --- a/gcc/analyzer/region-model.cc > +++ b/gcc/analyzer/region-model.cc > @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const > gswitch *switch_stmt, tree int_cst) > has nondefault cases handling all values in the enum. */ > > static bool > -has_nondefault_cases_for_all_enum_values_p (const gswitch > *switch_stmt) > +has_nondefault_cases_for_all_enum_values_p (const gswitch > *switch_stmt, > + tree type) > { > gcc_assert (switch_stmt); > - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); > gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); > > for (tree enum_val_iter = TYPE_VALUES (type); > @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const > switch_cfg_superedge &edge, > { > tree index = gimple_switch_index (switch_stmt); > const svalue *index_sval = get_rvalue (index, ctxt); > + bool check_index_type = true; > + > + /* With -fshort-enum, there may be a type cast. */ > + if (ctxt && index_sval->get_kind () == SK_UNARYOP > + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) > + { > + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> > (index_sval); > + if (unaryop->get_op () == NOP_EXPR > + && is_a <const initial_svalue *> (unaryop->get_arg ())) > + if (const initial_svalue *initvalop = (as_a <const > initial_svalue *> > + (unaryop->get_arg > ()))) > + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) > + { > + index_sval = initvalop; > + check_index_type = false; > + } > + } > > /* If we're switching based on an enum type, assume that the user > is only > working with values from the enum. Hence if this is an > @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const > switch_cfg_superedge &edge, > ctxt > /* Must be an enum value. */ > && index_sval->get_type () > - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE > + && (!check_index_type > + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) > && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE > /* If we have a constant, then we can check it directly. */ > && index_sval->get_kind () != SK_CONSTANT > && edge.implicitly_created_default_p () > - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) > + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, > + index_sval- > >get_type ()) > /* Don't do this if there's a chance that the index is > attacker-controlled. */ > && !ctxt->possibly_tainted_p (index_sval)) > diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c > b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c > new file mode 100644 > index 0000000000000..98f6d91f97481 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c > @@ -0,0 +1,141 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-fno-short-enums" } */ > +/* { dg-skip-if "default" { ! short_enums } } */ > + > +#include "analyzer-decls.h" > + > +/* Verify the handling of "switch (enum_value)". */ > + > +enum e > +{ > + E_VAL0, > + E_VAL1, > + E_VAL2 > +}; > + > +/* Verify that we assume that "switch (enum)" doesn't follow > implicit > + "default" if all enum values have cases */ > + > +int test_all_values_covered_implicit_default_1 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + case E_VAL2: > + return 1945; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > +} > + > +int test_all_values_covered_implicit_default_2 (enum e x) > +{ > + int result; > + switch (x) > + { > + case E_VAL0: > + result = 1066; > + break; > + case E_VAL1: > + result = 1776; > + break; > + case E_VAL2: > + result = 1945; > + break; > + } > + return result; /* { dg-bogus "uninitialized" } */ > +} > + > +/* Verify that we consider paths that use the implicit default when > not > + all enum values are covered by cases. */ > + > +int test_missing_values_implicit_default_1 (enum e x) > +{ > + switch (x) /* { dg-message "following 'default:' branch" } */ > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + } > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 0; > +} > + > +int test_missing_values_implicit_default_2 (enum e x) > +{ > + int result; > + switch (x) /* { dg-message "following 'default:' branch" } */ > + { > + case E_VAL0: > + result = 1066; > + break; > + case E_VAL1: > + result = 1776; > + break; > + } > + return result; /* { dg-warning "uninitialized" } */ > +} > + > +/* Verify that explicit "default" isn't rejected. */ > + > +int test_all_values_covered_explicit_default_1 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + case E_VAL2: > + return 1945; > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 0; > + } > +} > + > +int test_missing_values_explicit_default_1 (enum e x) > +{ > + switch (x) > + { > + default: > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > + > +int test_missing_values_explicit_default_2 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 1945; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > + > +int test_just_default (enum e x) > +{ > + switch (x) > + { > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 42; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > + > diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c > b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c > new file mode 100644 > index 0000000000000..384113fde5cbf > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c > @@ -0,0 +1,140 @@ > +/* { dg-do compile } */ > +/* { dg-additional-options "-fshort-enums" } */ > +/* { dg-skip-if "default" { short_enums } } */ > + > +#include "analyzer-decls.h" > + > +/* Verify the handling of "switch (enum_value)". */ > + > +enum e > +{ > + E_VAL0, > + E_VAL1, > + E_VAL2 > +}; > + > +/* Verify that we assume that "switch (enum)" doesn't follow > implicit > + "default" if all enum values have cases */ > + > +int test_all_values_covered_implicit_default_1 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + case E_VAL2: > + return 1945; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > +} > + > +int test_all_values_covered_implicit_default_2 (enum e x) > +{ > + int result; > + switch (x) > + { > + case E_VAL0: > + result = 1066; > + break; > + case E_VAL1: > + result = 1776; > + break; > + case E_VAL2: > + result = 1945; > + break; > + } > + return result; /* { dg-bogus "uninitialized" } */ > +} > + > +/* Verify that we consider paths that use the implicit default when > not > + all enum values are covered by cases. */ > + > +int test_missing_values_implicit_default_1 (enum e x) > +{ > + switch (x) /* { dg-message "following 'default:' branch" } */ > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + } > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 0; > +} > + > +int test_missing_values_implicit_default_2 (enum e x) > +{ > + int result; > + switch (x) /* { dg-message "following 'default:' branch" } */ > + { > + case E_VAL0: > + result = 1066; > + break; > + case E_VAL1: > + result = 1776; > + break; > + } > + return result; /* { dg-warning "uninitialized" } */ > +} > + > +/* Verify that explicit "default" isn't rejected. */ > + > +int test_all_values_covered_explicit_default_1 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + case E_VAL2: > + return 1945; > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 0; > + } > +} > + > +int test_missing_values_explicit_default_1 (enum e x) > +{ > + switch (x) > + { > + default: > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > + > +int test_missing_values_explicit_default_2 (enum e x) > +{ > + switch (x) > + { > + case E_VAL0: > + return 1066; > + case E_VAL1: > + return 1776; > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 1945; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > + > +int test_just_default (enum e x) > +{ > + switch (x) > + { > + default: > + __analyzer_dump_path (); /* { dg-message "path" } */ > + return 42; > + } > + __analyzer_dump_path (); /* { dg-bogus "path" } */ > + return 0; > +} > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] analyzer: deal with -fshort-enums 2023-12-06 22:22 ` David Malcolm @ 2024-02-07 16:21 ` Torbjorn SVENSSON 2024-02-22 8:51 ` [PING] " Torbjorn SVENSSON 0 siblings, 1 reply; 11+ messages in thread From: Torbjorn SVENSSON @ 2024-02-07 16:21 UTC (permalink / raw) To: David Malcolm, Alexandre Oliva; +Cc: gcc-patches, Yvan Roux Hi, Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to releases/gcc-13? Without this backport, I see these failures on arm-none-eabi: FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 26) FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 44) FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 34) FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 52) FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c -O0 (test for bogus messages, line 82) FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c -O0 (test for bogus messages, line 83) Kind regards, Torbjörn On 2023-12-06 23:22, David Malcolm wrote: > On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >> >>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>> do. >> >> Sorry it took me so long, here it is. I added two tests, so that, >> regardless of the defaults, we get both circumstances tested, without >> repetition. >> >> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >> install? > > Thanks for the updated patch. > > Looks good to me. > > Dave > >> >> >> analyzer: deal with -fshort-enums >> >> On platforms that enable -fshort-enums by default, various switch- >> enum >> analyzer tests fail, because apply_constraints_for_gswitch doesn't >> expect the integral promotion type cast. I've arranged for the code >> to cope with those casts. >> >> >> for gcc/analyzer/ChangeLog >> >> * region-model.cc (has_nondefault_case_for_value_p): Take >> enumerate type as a parameter. >> (region_model::apply_constraints_for_gswitch): Cope with >> integral promotion type casts. >> >> for gcc/testsuite/ChangeLog >> >> * gcc.dg/analyzer/switch-short-enum-1.c: New. >> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >> --- >> gcc/analyzer/region-model.cc | 27 +++- >> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >> ++++++++++++++++++++ >> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >> ++++++++++++++++++++ >> 3 files changed, 304 insertions(+), 4 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >> enum-1.c >> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >> 1.c >> >> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >> model.cc >> index 2157ad2578b85..6a7a8bc9f4884 100644 >> --- a/gcc/analyzer/region-model.cc >> +++ b/gcc/analyzer/region-model.cc >> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >> gswitch *switch_stmt, tree int_cst) >> has nondefault cases handling all values in the enum. */ >> >> static bool >> -has_nondefault_cases_for_all_enum_values_p (const gswitch >> *switch_stmt) >> +has_nondefault_cases_for_all_enum_values_p (const gswitch >> *switch_stmt, >> + tree type) >> { >> gcc_assert (switch_stmt); >> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >> >> for (tree enum_val_iter = TYPE_VALUES (type); >> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >> switch_cfg_superedge &edge, >> { >> tree index = gimple_switch_index (switch_stmt); >> const svalue *index_sval = get_rvalue (index, ctxt); >> + bool check_index_type = true; >> + >> + /* With -fshort-enum, there may be a type cast. */ >> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >> + { >> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >> (index_sval); >> + if (unaryop->get_op () == NOP_EXPR >> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >> + if (const initial_svalue *initvalop = (as_a <const >> initial_svalue *> >> + (unaryop->get_arg >> ()))) >> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >> + { >> + index_sval = initvalop; >> + check_index_type = false; >> + } >> + } >> >> /* If we're switching based on an enum type, assume that the user >> is only >> working with values from the enum. Hence if this is an >> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >> switch_cfg_superedge &edge, >> ctxt >> /* Must be an enum value. */ >> && index_sval->get_type () >> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >> + && (!check_index_type >> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >> /* If we have a constant, then we can check it directly. */ >> && index_sval->get_kind () != SK_CONSTANT >> && edge.implicitly_created_default_p () >> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >> + index_sval- >>> get_type ()) >> /* Don't do this if there's a chance that the index is >> attacker-controlled. */ >> && !ctxt->possibly_tainted_p (index_sval)) >> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >> new file mode 100644 >> index 0000000000000..98f6d91f97481 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >> @@ -0,0 +1,141 @@ >> +/* { dg-do compile } */ >> +/* { dg-additional-options "-fno-short-enums" } */ >> +/* { dg-skip-if "default" { ! short_enums } } */ >> + >> +#include "analyzer-decls.h" >> + >> +/* Verify the handling of "switch (enum_value)". */ >> + >> +enum e >> +{ >> + E_VAL0, >> + E_VAL1, >> + E_VAL2 >> +}; >> + >> +/* Verify that we assume that "switch (enum)" doesn't follow >> implicit >> + "default" if all enum values have cases */ >> + >> +int test_all_values_covered_implicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + case E_VAL2: >> + return 1945; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> +} >> + >> +int test_all_values_covered_implicit_default_2 (enum e x) >> +{ >> + int result; >> + switch (x) >> + { >> + case E_VAL0: >> + result = 1066; >> + break; >> + case E_VAL1: >> + result = 1776; >> + break; >> + case E_VAL2: >> + result = 1945; >> + break; >> + } >> + return result; /* { dg-bogus "uninitialized" } */ >> +} >> + >> +/* Verify that we consider paths that use the implicit default when >> not >> + all enum values are covered by cases. */ >> + >> +int test_missing_values_implicit_default_1 (enum e x) >> +{ >> + switch (x) /* { dg-message "following 'default:' branch" } */ >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + } >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 0; >> +} >> + >> +int test_missing_values_implicit_default_2 (enum e x) >> +{ >> + int result; >> + switch (x) /* { dg-message "following 'default:' branch" } */ >> + { >> + case E_VAL0: >> + result = 1066; >> + break; >> + case E_VAL1: >> + result = 1776; >> + break; >> + } >> + return result; /* { dg-warning "uninitialized" } */ >> +} >> + >> +/* Verify that explicit "default" isn't rejected. */ >> + >> +int test_all_values_covered_explicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + case E_VAL2: >> + return 1945; >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 0; >> + } >> +} >> + >> +int test_missing_values_explicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + default: >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> + >> +int test_missing_values_explicit_default_2 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 1945; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> + >> +int test_just_default (enum e x) >> +{ >> + switch (x) >> + { >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 42; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> + >> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >> new file mode 100644 >> index 0000000000000..384113fde5cbf >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >> @@ -0,0 +1,140 @@ >> +/* { dg-do compile } */ >> +/* { dg-additional-options "-fshort-enums" } */ >> +/* { dg-skip-if "default" { short_enums } } */ >> + >> +#include "analyzer-decls.h" >> + >> +/* Verify the handling of "switch (enum_value)". */ >> + >> +enum e >> +{ >> + E_VAL0, >> + E_VAL1, >> + E_VAL2 >> +}; >> + >> +/* Verify that we assume that "switch (enum)" doesn't follow >> implicit >> + "default" if all enum values have cases */ >> + >> +int test_all_values_covered_implicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + case E_VAL2: >> + return 1945; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> +} >> + >> +int test_all_values_covered_implicit_default_2 (enum e x) >> +{ >> + int result; >> + switch (x) >> + { >> + case E_VAL0: >> + result = 1066; >> + break; >> + case E_VAL1: >> + result = 1776; >> + break; >> + case E_VAL2: >> + result = 1945; >> + break; >> + } >> + return result; /* { dg-bogus "uninitialized" } */ >> +} >> + >> +/* Verify that we consider paths that use the implicit default when >> not >> + all enum values are covered by cases. */ >> + >> +int test_missing_values_implicit_default_1 (enum e x) >> +{ >> + switch (x) /* { dg-message "following 'default:' branch" } */ >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + } >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 0; >> +} >> + >> +int test_missing_values_implicit_default_2 (enum e x) >> +{ >> + int result; >> + switch (x) /* { dg-message "following 'default:' branch" } */ >> + { >> + case E_VAL0: >> + result = 1066; >> + break; >> + case E_VAL1: >> + result = 1776; >> + break; >> + } >> + return result; /* { dg-warning "uninitialized" } */ >> +} >> + >> +/* Verify that explicit "default" isn't rejected. */ >> + >> +int test_all_values_covered_explicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + case E_VAL2: >> + return 1945; >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 0; >> + } >> +} >> + >> +int test_missing_values_explicit_default_1 (enum e x) >> +{ >> + switch (x) >> + { >> + default: >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> + >> +int test_missing_values_explicit_default_2 (enum e x) >> +{ >> + switch (x) >> + { >> + case E_VAL0: >> + return 1066; >> + case E_VAL1: >> + return 1776; >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 1945; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> + >> +int test_just_default (enum e x) >> +{ >> + switch (x) >> + { >> + default: >> + __analyzer_dump_path (); /* { dg-message "path" } */ >> + return 42; >> + } >> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >> + return 0; >> +} >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING] Re: [PATCH] analyzer: deal with -fshort-enums 2024-02-07 16:21 ` Torbjorn SVENSSON @ 2024-02-22 8:51 ` Torbjorn SVENSSON 2024-03-08 9:14 ` [PING^2] " Torbjorn SVENSSON 0 siblings, 1 reply; 11+ messages in thread From: Torbjorn SVENSSON @ 2024-02-22 8:51 UTC (permalink / raw) To: David Malcolm, Alexandre Oliva; +Cc: gcc-patches, Yvan Roux Ping! Kind regards, Torbjörn On 2024-02-07 17:21, Torbjorn SVENSSON wrote: > Hi, > > Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to > releases/gcc-13? > > Without this backport, I see these failures on arm-none-eabi: > > FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 26) > FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 44) > FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 34) > FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 52) > FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c -O0 > (test for bogus messages, line 82) > FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c -O0 > (test for bogus messages, line 83) > > Kind regards, > Torbjörn > > > On 2023-12-06 23:22, David Malcolm wrote: >> On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >>> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>> >>>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>>> do. >>> >>> Sorry it took me so long, here it is. I added two tests, so that, >>> regardless of the defaults, we get both circumstances tested, without >>> repetition. >>> >>> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >>> install? >> >> Thanks for the updated patch. >> >> Looks good to me. >> >> Dave >> >>> >>> >>> analyzer: deal with -fshort-enums >>> >>> On platforms that enable -fshort-enums by default, various switch- >>> enum >>> analyzer tests fail, because apply_constraints_for_gswitch doesn't >>> expect the integral promotion type cast. I've arranged for the code >>> to cope with those casts. >>> >>> >>> for gcc/analyzer/ChangeLog >>> >>> * region-model.cc (has_nondefault_case_for_value_p): Take >>> enumerate type as a parameter. >>> (region_model::apply_constraints_for_gswitch): Cope with >>> integral promotion type casts. >>> >>> for gcc/testsuite/ChangeLog >>> >>> * gcc.dg/analyzer/switch-short-enum-1.c: New. >>> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >>> --- >>> gcc/analyzer/region-model.cc | 27 +++- >>> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >>> ++++++++++++++++++++ >>> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >>> ++++++++++++++++++++ >>> 3 files changed, 304 insertions(+), 4 deletions(-) >>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >>> enum-1.c >>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >>> 1.c >>> >>> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >>> model.cc >>> index 2157ad2578b85..6a7a8bc9f4884 100644 >>> --- a/gcc/analyzer/region-model.cc >>> +++ b/gcc/analyzer/region-model.cc >>> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >>> gswitch *switch_stmt, tree int_cst) >>> has nondefault cases handling all values in the enum. */ >>> static bool >>> -has_nondefault_cases_for_all_enum_values_p (const gswitch >>> *switch_stmt) >>> +has_nondefault_cases_for_all_enum_values_p (const gswitch >>> *switch_stmt, >>> + tree type) >>> { >>> gcc_assert (switch_stmt); >>> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >>> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >>> for (tree enum_val_iter = TYPE_VALUES (type); >>> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >>> switch_cfg_superedge &edge, >>> { >>> tree index = gimple_switch_index (switch_stmt); >>> const svalue *index_sval = get_rvalue (index, ctxt); >>> + bool check_index_type = true; >>> + >>> + /* With -fshort-enum, there may be a type cast. */ >>> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >>> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >>> + { >>> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >>> (index_sval); >>> + if (unaryop->get_op () == NOP_EXPR >>> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >>> + if (const initial_svalue *initvalop = (as_a <const >>> initial_svalue *> >>> + (unaryop->get_arg >>> ()))) >>> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >>> + { >>> + index_sval = initvalop; >>> + check_index_type = false; >>> + } >>> + } >>> /* If we're switching based on an enum type, assume that the user >>> is only >>> working with values from the enum. Hence if this is an >>> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >>> switch_cfg_superedge &edge, >>> ctxt >>> /* Must be an enum value. */ >>> && index_sval->get_type () >>> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >>> + && (!check_index_type >>> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >>> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >>> /* If we have a constant, then we can check it directly. */ >>> && index_sval->get_kind () != SK_CONSTANT >>> && edge.implicitly_created_default_p () >>> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >>> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >>> + index_sval- >>>> get_type ()) >>> /* Don't do this if there's a chance that the index is >>> attacker-controlled. */ >>> && !ctxt->possibly_tainted_p (index_sval)) >>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>> new file mode 100644 >>> index 0000000000000..98f6d91f97481 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>> @@ -0,0 +1,141 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-additional-options "-fno-short-enums" } */ >>> +/* { dg-skip-if "default" { ! short_enums } } */ >>> + >>> +#include "analyzer-decls.h" >>> + >>> +/* Verify the handling of "switch (enum_value)". */ >>> + >>> +enum e >>> +{ >>> + E_VAL0, >>> + E_VAL1, >>> + E_VAL2 >>> +}; >>> + >>> +/* Verify that we assume that "switch (enum)" doesn't follow >>> implicit >>> + "default" if all enum values have cases */ >>> + >>> +int test_all_values_covered_implicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + case E_VAL2: >>> + return 1945; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> +} >>> + >>> +int test_all_values_covered_implicit_default_2 (enum e x) >>> +{ >>> + int result; >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + result = 1066; >>> + break; >>> + case E_VAL1: >>> + result = 1776; >>> + break; >>> + case E_VAL2: >>> + result = 1945; >>> + break; >>> + } >>> + return result; /* { dg-bogus "uninitialized" } */ >>> +} >>> + >>> +/* Verify that we consider paths that use the implicit default when >>> not >>> + all enum values are covered by cases. */ >>> + >>> +int test_missing_values_implicit_default_1 (enum e x) >>> +{ >>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + } >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_missing_values_implicit_default_2 (enum e x) >>> +{ >>> + int result; >>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>> + { >>> + case E_VAL0: >>> + result = 1066; >>> + break; >>> + case E_VAL1: >>> + result = 1776; >>> + break; >>> + } >>> + return result; /* { dg-warning "uninitialized" } */ >>> +} >>> + >>> +/* Verify that explicit "default" isn't rejected. */ >>> + >>> +int test_all_values_covered_explicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + case E_VAL2: >>> + return 1945; >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 0; >>> + } >>> +} >>> + >>> +int test_missing_values_explicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + default: >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_missing_values_explicit_default_2 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 1945; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_just_default (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 42; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> + >>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>> new file mode 100644 >>> index 0000000000000..384113fde5cbf >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>> @@ -0,0 +1,140 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-additional-options "-fshort-enums" } */ >>> +/* { dg-skip-if "default" { short_enums } } */ >>> + >>> +#include "analyzer-decls.h" >>> + >>> +/* Verify the handling of "switch (enum_value)". */ >>> + >>> +enum e >>> +{ >>> + E_VAL0, >>> + E_VAL1, >>> + E_VAL2 >>> +}; >>> + >>> +/* Verify that we assume that "switch (enum)" doesn't follow >>> implicit >>> + "default" if all enum values have cases */ >>> + >>> +int test_all_values_covered_implicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + case E_VAL2: >>> + return 1945; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> +} >>> + >>> +int test_all_values_covered_implicit_default_2 (enum e x) >>> +{ >>> + int result; >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + result = 1066; >>> + break; >>> + case E_VAL1: >>> + result = 1776; >>> + break; >>> + case E_VAL2: >>> + result = 1945; >>> + break; >>> + } >>> + return result; /* { dg-bogus "uninitialized" } */ >>> +} >>> + >>> +/* Verify that we consider paths that use the implicit default when >>> not >>> + all enum values are covered by cases. */ >>> + >>> +int test_missing_values_implicit_default_1 (enum e x) >>> +{ >>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + } >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_missing_values_implicit_default_2 (enum e x) >>> +{ >>> + int result; >>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>> + { >>> + case E_VAL0: >>> + result = 1066; >>> + break; >>> + case E_VAL1: >>> + result = 1776; >>> + break; >>> + } >>> + return result; /* { dg-warning "uninitialized" } */ >>> +} >>> + >>> +/* Verify that explicit "default" isn't rejected. */ >>> + >>> +int test_all_values_covered_explicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + case E_VAL2: >>> + return 1945; >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 0; >>> + } >>> +} >>> + >>> +int test_missing_values_explicit_default_1 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + default: >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_missing_values_explicit_default_2 (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + case E_VAL0: >>> + return 1066; >>> + case E_VAL1: >>> + return 1776; >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 1945; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> + >>> +int test_just_default (enum e x) >>> +{ >>> + switch (x) >>> + { >>> + default: >>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>> + return 42; >>> + } >>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>> + return 0; >>> +} >>> >>> >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING^2] Re: [PATCH] analyzer: deal with -fshort-enums 2024-02-22 8:51 ` [PING] " Torbjorn SVENSSON @ 2024-03-08 9:14 ` Torbjorn SVENSSON 2024-03-15 10:32 ` [PING^3] " Torbjorn SVENSSON 0 siblings, 1 reply; 11+ messages in thread From: Torbjorn SVENSSON @ 2024-03-08 9:14 UTC (permalink / raw) To: David Malcolm, Alexandre Oliva; +Cc: gcc-patches, Yvan Roux Ping! Kind regards, Torbjörn On 2024-02-22 09:51, Torbjorn SVENSSON wrote: > Ping! > > Kind regards, > Torbjörn > > On 2024-02-07 17:21, Torbjorn SVENSSON wrote: >> Hi, >> >> Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to >> releases/gcc-13? >> >> Without this backport, I see these failures on arm-none-eabi: >> >> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 26) >> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line 44) >> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 34) >> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line 52) >> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c >> -O0 (test for bogus messages, line 82) >> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c >> -O0 (test for bogus messages, line 83) >> >> Kind regards, >> Torbjörn >> >> >> On 2023-12-06 23:22, David Malcolm wrote: >>> On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >>>> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>>> >>>>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>>>> do. >>>> >>>> Sorry it took me so long, here it is. I added two tests, so that, >>>> regardless of the defaults, we get both circumstances tested, without >>>> repetition. >>>> >>>> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >>>> install? >>> >>> Thanks for the updated patch. >>> >>> Looks good to me. >>> >>> Dave >>> >>>> >>>> >>>> analyzer: deal with -fshort-enums >>>> >>>> On platforms that enable -fshort-enums by default, various switch- >>>> enum >>>> analyzer tests fail, because apply_constraints_for_gswitch doesn't >>>> expect the integral promotion type cast. I've arranged for the code >>>> to cope with those casts. >>>> >>>> >>>> for gcc/analyzer/ChangeLog >>>> >>>> * region-model.cc (has_nondefault_case_for_value_p): Take >>>> enumerate type as a parameter. >>>> (region_model::apply_constraints_for_gswitch): Cope with >>>> integral promotion type casts. >>>> >>>> for gcc/testsuite/ChangeLog >>>> >>>> * gcc.dg/analyzer/switch-short-enum-1.c: New. >>>> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >>>> --- >>>> gcc/analyzer/region-model.cc | 27 +++- >>>> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >>>> ++++++++++++++++++++ >>>> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >>>> ++++++++++++++++++++ >>>> 3 files changed, 304 insertions(+), 4 deletions(-) >>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >>>> enum-1.c >>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >>>> 1.c >>>> >>>> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >>>> model.cc >>>> index 2157ad2578b85..6a7a8bc9f4884 100644 >>>> --- a/gcc/analyzer/region-model.cc >>>> +++ b/gcc/analyzer/region-model.cc >>>> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >>>> gswitch *switch_stmt, tree int_cst) >>>> has nondefault cases handling all values in the enum. */ >>>> static bool >>>> -has_nondefault_cases_for_all_enum_values_p (const gswitch >>>> *switch_stmt) >>>> +has_nondefault_cases_for_all_enum_values_p (const gswitch >>>> *switch_stmt, >>>> + tree type) >>>> { >>>> gcc_assert (switch_stmt); >>>> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >>>> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >>>> for (tree enum_val_iter = TYPE_VALUES (type); >>>> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >>>> switch_cfg_superedge &edge, >>>> { >>>> tree index = gimple_switch_index (switch_stmt); >>>> const svalue *index_sval = get_rvalue (index, ctxt); >>>> + bool check_index_type = true; >>>> + >>>> + /* With -fshort-enum, there may be a type cast. */ >>>> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >>>> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >>>> + { >>>> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >>>> (index_sval); >>>> + if (unaryop->get_op () == NOP_EXPR >>>> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >>>> + if (const initial_svalue *initvalop = (as_a <const >>>> initial_svalue *> >>>> + (unaryop->get_arg >>>> ()))) >>>> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >>>> + { >>>> + index_sval = initvalop; >>>> + check_index_type = false; >>>> + } >>>> + } >>>> /* If we're switching based on an enum type, assume that the user >>>> is only >>>> working with values from the enum. Hence if this is an >>>> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >>>> switch_cfg_superedge &edge, >>>> ctxt >>>> /* Must be an enum value. */ >>>> && index_sval->get_type () >>>> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >>>> + && (!check_index_type >>>> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >>>> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >>>> /* If we have a constant, then we can check it directly. */ >>>> && index_sval->get_kind () != SK_CONSTANT >>>> && edge.implicitly_created_default_p () >>>> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >>>> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >>>> + index_sval- >>>>> get_type ()) >>>> /* Don't do this if there's a chance that the index is >>>> attacker-controlled. */ >>>> && !ctxt->possibly_tainted_p (index_sval)) >>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>> new file mode 100644 >>>> index 0000000000000..98f6d91f97481 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>> @@ -0,0 +1,141 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-additional-options "-fno-short-enums" } */ >>>> +/* { dg-skip-if "default" { ! short_enums } } */ >>>> + >>>> +#include "analyzer-decls.h" >>>> + >>>> +/* Verify the handling of "switch (enum_value)". */ >>>> + >>>> +enum e >>>> +{ >>>> + E_VAL0, >>>> + E_VAL1, >>>> + E_VAL2 >>>> +}; >>>> + >>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>> implicit >>>> + "default" if all enum values have cases */ >>>> + >>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + case E_VAL2: >>>> + return 1945; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> +} >>>> + >>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>> +{ >>>> + int result; >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + result = 1066; >>>> + break; >>>> + case E_VAL1: >>>> + result = 1776; >>>> + break; >>>> + case E_VAL2: >>>> + result = 1945; >>>> + break; >>>> + } >>>> + return result; /* { dg-bogus "uninitialized" } */ >>>> +} >>>> + >>>> +/* Verify that we consider paths that use the implicit default when >>>> not >>>> + all enum values are covered by cases. */ >>>> + >>>> +int test_missing_values_implicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_missing_values_implicit_default_2 (enum e x) >>>> +{ >>>> + int result; >>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>> + { >>>> + case E_VAL0: >>>> + result = 1066; >>>> + break; >>>> + case E_VAL1: >>>> + result = 1776; >>>> + break; >>>> + } >>>> + return result; /* { dg-warning "uninitialized" } */ >>>> +} >>>> + >>>> +/* Verify that explicit "default" isn't rejected. */ >>>> + >>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + case E_VAL2: >>>> + return 1945; >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 0; >>>> + } >>>> +} >>>> + >>>> +int test_missing_values_explicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + default: >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_missing_values_explicit_default_2 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 1945; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_just_default (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 42; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>> new file mode 100644 >>>> index 0000000000000..384113fde5cbf >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>> @@ -0,0 +1,140 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-additional-options "-fshort-enums" } */ >>>> +/* { dg-skip-if "default" { short_enums } } */ >>>> + >>>> +#include "analyzer-decls.h" >>>> + >>>> +/* Verify the handling of "switch (enum_value)". */ >>>> + >>>> +enum e >>>> +{ >>>> + E_VAL0, >>>> + E_VAL1, >>>> + E_VAL2 >>>> +}; >>>> + >>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>> implicit >>>> + "default" if all enum values have cases */ >>>> + >>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + case E_VAL2: >>>> + return 1945; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> +} >>>> + >>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>> +{ >>>> + int result; >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + result = 1066; >>>> + break; >>>> + case E_VAL1: >>>> + result = 1776; >>>> + break; >>>> + case E_VAL2: >>>> + result = 1945; >>>> + break; >>>> + } >>>> + return result; /* { dg-bogus "uninitialized" } */ >>>> +} >>>> + >>>> +/* Verify that we consider paths that use the implicit default when >>>> not >>>> + all enum values are covered by cases. */ >>>> + >>>> +int test_missing_values_implicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_missing_values_implicit_default_2 (enum e x) >>>> +{ >>>> + int result; >>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>> + { >>>> + case E_VAL0: >>>> + result = 1066; >>>> + break; >>>> + case E_VAL1: >>>> + result = 1776; >>>> + break; >>>> + } >>>> + return result; /* { dg-warning "uninitialized" } */ >>>> +} >>>> + >>>> +/* Verify that explicit "default" isn't rejected. */ >>>> + >>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + case E_VAL2: >>>> + return 1945; >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 0; >>>> + } >>>> +} >>>> + >>>> +int test_missing_values_explicit_default_1 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + default: >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_missing_values_explicit_default_2 (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + case E_VAL0: >>>> + return 1066; >>>> + case E_VAL1: >>>> + return 1776; >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 1945; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> + >>>> +int test_just_default (enum e x) >>>> +{ >>>> + switch (x) >>>> + { >>>> + default: >>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>> + return 42; >>>> + } >>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>> + return 0; >>>> +} >>>> >>>> >>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING^3] Re: [PATCH] analyzer: deal with -fshort-enums 2024-03-08 9:14 ` [PING^2] " Torbjorn SVENSSON @ 2024-03-15 10:32 ` Torbjorn SVENSSON 2024-03-25 14:59 ` [PING^4] " Yvan ROUX - foss 0 siblings, 1 reply; 11+ messages in thread From: Torbjorn SVENSSON @ 2024-03-15 10:32 UTC (permalink / raw) To: David Malcolm, Alexandre Oliva; +Cc: gcc-patches, Yvan Roux Ping! Kind regards, Torbjörn On 2024-03-08 10:14, Torbjorn SVENSSON wrote: > Ping! > > Kind regards, > Torbjörn > > On 2024-02-22 09:51, Torbjorn SVENSSON wrote: >> Ping! >> >> Kind regards, >> Torbjörn >> >> On 2024-02-07 17:21, Torbjorn SVENSSON wrote: >>> Hi, >>> >>> Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to >>> releases/gcc-13? >>> >>> Without this backport, I see these failures on arm-none-eabi: >>> >>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>> 26) >>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>> 44) >>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>> 34) >>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>> 52) >>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c -O0 >>> (test for bogus messages, line 82) >>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c >>> -O0 (test for bogus messages, line 83) >>> >>> Kind regards, >>> Torbjörn >>> >>> >>> On 2023-12-06 23:22, David Malcolm wrote: >>>> On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >>>>> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>>>> >>>>>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>>>>> do. >>>>> >>>>> Sorry it took me so long, here it is. I added two tests, so that, >>>>> regardless of the defaults, we get both circumstances tested, without >>>>> repetition. >>>>> >>>>> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >>>>> install? >>>> >>>> Thanks for the updated patch. >>>> >>>> Looks good to me. >>>> >>>> Dave >>>> >>>>> >>>>> >>>>> analyzer: deal with -fshort-enums >>>>> >>>>> On platforms that enable -fshort-enums by default, various switch- >>>>> enum >>>>> analyzer tests fail, because apply_constraints_for_gswitch doesn't >>>>> expect the integral promotion type cast. I've arranged for the code >>>>> to cope with those casts. >>>>> >>>>> >>>>> for gcc/analyzer/ChangeLog >>>>> >>>>> * region-model.cc (has_nondefault_case_for_value_p): Take >>>>> enumerate type as a parameter. >>>>> (region_model::apply_constraints_for_gswitch): Cope with >>>>> integral promotion type casts. >>>>> >>>>> for gcc/testsuite/ChangeLog >>>>> >>>>> * gcc.dg/analyzer/switch-short-enum-1.c: New. >>>>> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >>>>> --- >>>>> gcc/analyzer/region-model.cc | 27 +++- >>>>> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >>>>> ++++++++++++++++++++ >>>>> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >>>>> ++++++++++++++++++++ >>>>> 3 files changed, 304 insertions(+), 4 deletions(-) >>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >>>>> enum-1.c >>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >>>>> 1.c >>>>> >>>>> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >>>>> model.cc >>>>> index 2157ad2578b85..6a7a8bc9f4884 100644 >>>>> --- a/gcc/analyzer/region-model.cc >>>>> +++ b/gcc/analyzer/region-model.cc >>>>> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >>>>> gswitch *switch_stmt, tree int_cst) >>>>> has nondefault cases handling all values in the enum. */ >>>>> static bool >>>>> -has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>> *switch_stmt) >>>>> +has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>> *switch_stmt, >>>>> + tree type) >>>>> { >>>>> gcc_assert (switch_stmt); >>>>> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >>>>> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >>>>> for (tree enum_val_iter = TYPE_VALUES (type); >>>>> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >>>>> switch_cfg_superedge &edge, >>>>> { >>>>> tree index = gimple_switch_index (switch_stmt); >>>>> const svalue *index_sval = get_rvalue (index, ctxt); >>>>> + bool check_index_type = true; >>>>> + >>>>> + /* With -fshort-enum, there may be a type cast. */ >>>>> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >>>>> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >>>>> + { >>>>> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >>>>> (index_sval); >>>>> + if (unaryop->get_op () == NOP_EXPR >>>>> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >>>>> + if (const initial_svalue *initvalop = (as_a <const >>>>> initial_svalue *> >>>>> + (unaryop->get_arg >>>>> ()))) >>>>> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >>>>> + { >>>>> + index_sval = initvalop; >>>>> + check_index_type = false; >>>>> + } >>>>> + } >>>>> /* If we're switching based on an enum type, assume that the user >>>>> is only >>>>> working with values from the enum. Hence if this is an >>>>> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >>>>> switch_cfg_superedge &edge, >>>>> ctxt >>>>> /* Must be an enum value. */ >>>>> && index_sval->get_type () >>>>> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >>>>> + && (!check_index_type >>>>> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >>>>> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >>>>> /* If we have a constant, then we can check it directly. */ >>>>> && index_sval->get_kind () != SK_CONSTANT >>>>> && edge.implicitly_created_default_p () >>>>> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >>>>> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >>>>> + index_sval- >>>>>> get_type ()) >>>>> /* Don't do this if there's a chance that the index is >>>>> attacker-controlled. */ >>>>> && !ctxt->possibly_tainted_p (index_sval)) >>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> new file mode 100644 >>>>> index 0000000000000..98f6d91f97481 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> @@ -0,0 +1,141 @@ >>>>> +/* { dg-do compile } */ >>>>> +/* { dg-additional-options "-fno-short-enums" } */ >>>>> +/* { dg-skip-if "default" { ! short_enums } } */ >>>>> + >>>>> +#include "analyzer-decls.h" >>>>> + >>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>> + >>>>> +enum e >>>>> +{ >>>>> + E_VAL0, >>>>> + E_VAL1, >>>>> + E_VAL2 >>>>> +}; >>>>> + >>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>> implicit >>>>> + "default" if all enum values have cases */ >>>>> + >>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> +} >>>>> + >>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + case E_VAL2: >>>>> + result = 1945; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that we consider paths that use the implicit default when >>>>> not >>>>> + all enum values are covered by cases. */ >>>>> + >>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>> + >>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> + } >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_just_default (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 42; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> new file mode 100644 >>>>> index 0000000000000..384113fde5cbf >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> @@ -0,0 +1,140 @@ >>>>> +/* { dg-do compile } */ >>>>> +/* { dg-additional-options "-fshort-enums" } */ >>>>> +/* { dg-skip-if "default" { short_enums } } */ >>>>> + >>>>> +#include "analyzer-decls.h" >>>>> + >>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>> + >>>>> +enum e >>>>> +{ >>>>> + E_VAL0, >>>>> + E_VAL1, >>>>> + E_VAL2 >>>>> +}; >>>>> + >>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>> implicit >>>>> + "default" if all enum values have cases */ >>>>> + >>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> +} >>>>> + >>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + case E_VAL2: >>>>> + result = 1945; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that we consider paths that use the implicit default when >>>>> not >>>>> + all enum values are covered by cases. */ >>>>> + >>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>> + >>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> + } >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_just_default (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 42; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> >>>>> >>>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING^4] Re: [PATCH] analyzer: deal with -fshort-enums 2024-03-15 10:32 ` [PING^3] " Torbjorn SVENSSON @ 2024-03-25 14:59 ` Yvan ROUX - foss 2024-04-10 13:23 ` [PING^5] " Torbjorn SVENSSON 0 siblings, 1 reply; 11+ messages in thread From: Yvan ROUX - foss @ 2024-03-25 14:59 UTC (permalink / raw) To: Torbjorn SVENSSON - foss, David Malcolm, Alexandre Oliva; +Cc: gcc-patches Ping! Rgds, Yvan ________________________________________ From: Torbjorn SVENSSON - foss Sent: Friday, March 15, 2024 11:32 AM To: David Malcolm; Alexandre Oliva Cc: gcc-patches@gcc.gnu.org; Yvan ROUX - foss Subject: [PING^3] Re: [PATCH] analyzer: deal with -fshort-enums Ping! Kind regards, Torbjörn On 2024-03-08 10:14, Torbjorn SVENSSON wrote: > Ping! > > Kind regards, > Torbjörn > > On 2024-02-22 09:51, Torbjorn SVENSSON wrote: >> Ping! >> >> Kind regards, >> Torbjörn >> >> On 2024-02-07 17:21, Torbjorn SVENSSON wrote: >>> Hi, >>> >>> Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to >>> releases/gcc-13? >>> >>> Without this backport, I see these failures on arm-none-eabi: >>> >>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>> 26) >>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>> 44) >>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>> 34) >>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>> 52) >>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c -O0 >>> (test for bogus messages, line 82) >>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c >>> -O0 (test for bogus messages, line 83) >>> >>> Kind regards, >>> Torbjörn >>> >>> >>> On 2023-12-06 23:22, David Malcolm wrote: >>>> On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >>>>> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>>>> >>>>>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>>>>> do. >>>>> >>>>> Sorry it took me so long, here it is. I added two tests, so that, >>>>> regardless of the defaults, we get both circumstances tested, without >>>>> repetition. >>>>> >>>>> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >>>>> install? >>>> >>>> Thanks for the updated patch. >>>> >>>> Looks good to me. >>>> >>>> Dave >>>> >>>>> >>>>> >>>>> analyzer: deal with -fshort-enums >>>>> >>>>> On platforms that enable -fshort-enums by default, various switch- >>>>> enum >>>>> analyzer tests fail, because apply_constraints_for_gswitch doesn't >>>>> expect the integral promotion type cast. I've arranged for the code >>>>> to cope with those casts. >>>>> >>>>> >>>>> for gcc/analyzer/ChangeLog >>>>> >>>>> * region-model.cc (has_nondefault_case_for_value_p): Take >>>>> enumerate type as a parameter. >>>>> (region_model::apply_constraints_for_gswitch): Cope with >>>>> integral promotion type casts. >>>>> >>>>> for gcc/testsuite/ChangeLog >>>>> >>>>> * gcc.dg/analyzer/switch-short-enum-1.c: New. >>>>> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >>>>> --- >>>>> gcc/analyzer/region-model.cc | 27 +++- >>>>> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >>>>> ++++++++++++++++++++ >>>>> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >>>>> ++++++++++++++++++++ >>>>> 3 files changed, 304 insertions(+), 4 deletions(-) >>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >>>>> enum-1.c >>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >>>>> 1.c >>>>> >>>>> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >>>>> model.cc >>>>> index 2157ad2578b85..6a7a8bc9f4884 100644 >>>>> --- a/gcc/analyzer/region-model.cc >>>>> +++ b/gcc/analyzer/region-model.cc >>>>> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >>>>> gswitch *switch_stmt, tree int_cst) >>>>> has nondefault cases handling all values in the enum. */ >>>>> static bool >>>>> -has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>> *switch_stmt) >>>>> +has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>> *switch_stmt, >>>>> + tree type) >>>>> { >>>>> gcc_assert (switch_stmt); >>>>> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >>>>> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >>>>> for (tree enum_val_iter = TYPE_VALUES (type); >>>>> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >>>>> switch_cfg_superedge &edge, >>>>> { >>>>> tree index = gimple_switch_index (switch_stmt); >>>>> const svalue *index_sval = get_rvalue (index, ctxt); >>>>> + bool check_index_type = true; >>>>> + >>>>> + /* With -fshort-enum, there may be a type cast. */ >>>>> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >>>>> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >>>>> + { >>>>> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >>>>> (index_sval); >>>>> + if (unaryop->get_op () == NOP_EXPR >>>>> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >>>>> + if (const initial_svalue *initvalop = (as_a <const >>>>> initial_svalue *> >>>>> + (unaryop->get_arg >>>>> ()))) >>>>> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >>>>> + { >>>>> + index_sval = initvalop; >>>>> + check_index_type = false; >>>>> + } >>>>> + } >>>>> /* If we're switching based on an enum type, assume that the user >>>>> is only >>>>> working with values from the enum. Hence if this is an >>>>> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >>>>> switch_cfg_superedge &edge, >>>>> ctxt >>>>> /* Must be an enum value. */ >>>>> && index_sval->get_type () >>>>> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >>>>> + && (!check_index_type >>>>> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >>>>> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >>>>> /* If we have a constant, then we can check it directly. */ >>>>> && index_sval->get_kind () != SK_CONSTANT >>>>> && edge.implicitly_created_default_p () >>>>> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >>>>> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >>>>> + index_sval- >>>>>> get_type ()) >>>>> /* Don't do this if there's a chance that the index is >>>>> attacker-controlled. */ >>>>> && !ctxt->possibly_tainted_p (index_sval)) >>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> new file mode 100644 >>>>> index 0000000000000..98f6d91f97481 >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>> @@ -0,0 +1,141 @@ >>>>> +/* { dg-do compile } */ >>>>> +/* { dg-additional-options "-fno-short-enums" } */ >>>>> +/* { dg-skip-if "default" { ! short_enums } } */ >>>>> + >>>>> +#include "analyzer-decls.h" >>>>> + >>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>> + >>>>> +enum e >>>>> +{ >>>>> + E_VAL0, >>>>> + E_VAL1, >>>>> + E_VAL2 >>>>> +}; >>>>> + >>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>> implicit >>>>> + "default" if all enum values have cases */ >>>>> + >>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> +} >>>>> + >>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + case E_VAL2: >>>>> + result = 1945; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that we consider paths that use the implicit default when >>>>> not >>>>> + all enum values are covered by cases. */ >>>>> + >>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>> + >>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> + } >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_just_default (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 42; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> new file mode 100644 >>>>> index 0000000000000..384113fde5cbf >>>>> --- /dev/null >>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>> @@ -0,0 +1,140 @@ >>>>> +/* { dg-do compile } */ >>>>> +/* { dg-additional-options "-fshort-enums" } */ >>>>> +/* { dg-skip-if "default" { short_enums } } */ >>>>> + >>>>> +#include "analyzer-decls.h" >>>>> + >>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>> + >>>>> +enum e >>>>> +{ >>>>> + E_VAL0, >>>>> + E_VAL1, >>>>> + E_VAL2 >>>>> +}; >>>>> + >>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>> implicit >>>>> + "default" if all enum values have cases */ >>>>> + >>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> +} >>>>> + >>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + case E_VAL2: >>>>> + result = 1945; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that we consider paths that use the implicit default when >>>>> not >>>>> + all enum values are covered by cases. */ >>>>> + >>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>> +{ >>>>> + int result; >>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>> + { >>>>> + case E_VAL0: >>>>> + result = 1066; >>>>> + break; >>>>> + case E_VAL1: >>>>> + result = 1776; >>>>> + break; >>>>> + } >>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>> +} >>>>> + >>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>> + >>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + case E_VAL2: >>>>> + return 1945; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 0; >>>>> + } >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + case E_VAL0: >>>>> + return 1066; >>>>> + case E_VAL1: >>>>> + return 1776; >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 1945; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> + >>>>> +int test_just_default (enum e x) >>>>> +{ >>>>> + switch (x) >>>>> + { >>>>> + default: >>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>> + return 42; >>>>> + } >>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>> + return 0; >>>>> +} >>>>> >>>>> >>>> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING^5] Re: [PATCH] analyzer: deal with -fshort-enums 2024-03-25 14:59 ` [PING^4] " Yvan ROUX - foss @ 2024-04-10 13:23 ` Torbjorn SVENSSON 0 siblings, 0 replies; 11+ messages in thread From: Torbjorn SVENSSON @ 2024-04-10 13:23 UTC (permalink / raw) To: Yvan ROUX - foss, David Malcolm, Alexandre Oliva, Richard Biener Cc: gcc-patches Ping! Kind regards, Torbjörn On 2024-03-25 15:59, Yvan ROUX - foss wrote: > Ping! > > Rgds, > Yvan > ________________________________________ > From: Torbjorn SVENSSON - foss > Sent: Friday, March 15, 2024 11:32 AM > To: David Malcolm; Alexandre Oliva > Cc: gcc-patches@gcc.gnu.org; Yvan ROUX - foss > Subject: [PING^3] Re: [PATCH] analyzer: deal with -fshort-enums > > Ping! > > Kind regards, > Torbjörn > > On 2024-03-08 10:14, Torbjorn SVENSSON wrote: >> Ping! >> >> Kind regards, >> Torbjörn >> >> On 2024-02-22 09:51, Torbjorn SVENSSON wrote: >>> Ping! >>> >>> Kind regards, >>> Torbjörn >>> >>> On 2024-02-07 17:21, Torbjorn SVENSSON wrote: >>>> Hi, >>>> >>>> Is it okay to backport 3cbab07b08d2f3a3ed34b6ec12e67727c59d285c to >>>> releases/gcc-13? >>>> >>>> Without this backport, I see these failures on arm-none-eabi: >>>> >>>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>>> 26) >>>> FAIL: gcc.dg/analyzer/switch-enum-1.c (test for bogus messages, line >>>> 44) >>>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>>> 34) >>>> FAIL: gcc.dg/analyzer/switch-enum-2.c (test for bogus messages, line >>>> 52) >>>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_floor.c -O0 >>>> (test for bogus messages, line 82) >>>> FAIL: gcc.dg/analyzer/torture/switch-enum-pr105273-doom-p_maputl.c >>>> -O0 (test for bogus messages, line 83) >>>> >>>> Kind regards, >>>> Torbjörn >>>> >>>> >>>> On 2023-12-06 23:22, David Malcolm wrote: >>>>> On Wed, 2023-12-06 at 02:31 -0300, Alexandre Oliva wrote: >>>>>> On Nov 22, 2023, Alexandre Oliva <oliva@adacore.com> wrote: >>>>>> >>>>>>> Ah, nice, that's a great idea, I wish I'd thought of that! Will >>>>>>> do. >>>>>> >>>>>> Sorry it took me so long, here it is. I added two tests, so that, >>>>>> regardless of the defaults, we get both circumstances tested, without >>>>>> repetition. >>>>>> >>>>>> Regstrapped on x86_64-linux-gnu. Also tested on arm-eabi. Ok to >>>>>> install? >>>>> >>>>> Thanks for the updated patch. >>>>> >>>>> Looks good to me. >>>>> >>>>> Dave >>>>> >>>>>> >>>>>> >>>>>> analyzer: deal with -fshort-enums >>>>>> >>>>>> On platforms that enable -fshort-enums by default, various switch- >>>>>> enum >>>>>> analyzer tests fail, because apply_constraints_for_gswitch doesn't >>>>>> expect the integral promotion type cast. I've arranged for the code >>>>>> to cope with those casts. >>>>>> >>>>>> >>>>>> for gcc/analyzer/ChangeLog >>>>>> >>>>>> * region-model.cc (has_nondefault_case_for_value_p): Take >>>>>> enumerate type as a parameter. >>>>>> (region_model::apply_constraints_for_gswitch): Cope with >>>>>> integral promotion type casts. >>>>>> >>>>>> for gcc/testsuite/ChangeLog >>>>>> >>>>>> * gcc.dg/analyzer/switch-short-enum-1.c: New. >>>>>> * gcc.dg/analyzer/switch-no-short-enum-1.c: New. >>>>>> --- >>>>>> gcc/analyzer/region-model.cc | 27 +++- >>>>>> .../gcc.dg/analyzer/switch-no-short-enum-1.c | 141 >>>>>> ++++++++++++++++++++ >>>>>> .../gcc.dg/analyzer/switch-short-enum-1.c | 140 >>>>>> ++++++++++++++++++++ >>>>>> 3 files changed, 304 insertions(+), 4 deletions(-) >>>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-no-short- >>>>>> enum-1.c >>>>>> create mode 100644 gcc/testsuite/gcc.dg/analyzer/switch-short-enum- >>>>>> 1.c >>>>>> >>>>>> diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region- >>>>>> model.cc >>>>>> index 2157ad2578b85..6a7a8bc9f4884 100644 >>>>>> --- a/gcc/analyzer/region-model.cc >>>>>> +++ b/gcc/analyzer/region-model.cc >>>>>> @@ -5387,10 +5387,10 @@ has_nondefault_case_for_value_p (const >>>>>> gswitch *switch_stmt, tree int_cst) >>>>>> has nondefault cases handling all values in the enum. */ >>>>>> static bool >>>>>> -has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>>> *switch_stmt) >>>>>> +has_nondefault_cases_for_all_enum_values_p (const gswitch >>>>>> *switch_stmt, >>>>>> + tree type) >>>>>> { >>>>>> gcc_assert (switch_stmt); >>>>>> - tree type = TREE_TYPE (gimple_switch_index (switch_stmt)); >>>>>> gcc_assert (TREE_CODE (type) == ENUMERAL_TYPE); >>>>>> for (tree enum_val_iter = TYPE_VALUES (type); >>>>>> @@ -5426,6 +5426,23 @@ apply_constraints_for_gswitch (const >>>>>> switch_cfg_superedge &edge, >>>>>> { >>>>>> tree index = gimple_switch_index (switch_stmt); >>>>>> const svalue *index_sval = get_rvalue (index, ctxt); >>>>>> + bool check_index_type = true; >>>>>> + >>>>>> + /* With -fshort-enum, there may be a type cast. */ >>>>>> + if (ctxt && index_sval->get_kind () == SK_UNARYOP >>>>>> + && TREE_CODE (index_sval->get_type ()) == INTEGER_TYPE) >>>>>> + { >>>>>> + const unaryop_svalue *unaryop = as_a <const unaryop_svalue *> >>>>>> (index_sval); >>>>>> + if (unaryop->get_op () == NOP_EXPR >>>>>> + && is_a <const initial_svalue *> (unaryop->get_arg ())) >>>>>> + if (const initial_svalue *initvalop = (as_a <const >>>>>> initial_svalue *> >>>>>> + (unaryop->get_arg >>>>>> ()))) >>>>>> + if (TREE_CODE (initvalop->get_type ()) == ENUMERAL_TYPE) >>>>>> + { >>>>>> + index_sval = initvalop; >>>>>> + check_index_type = false; >>>>>> + } >>>>>> + } >>>>>> /* If we're switching based on an enum type, assume that the user >>>>>> is only >>>>>> working with values from the enum. Hence if this is an >>>>>> @@ -5437,12 +5454,14 @@ apply_constraints_for_gswitch (const >>>>>> switch_cfg_superedge &edge, >>>>>> ctxt >>>>>> /* Must be an enum value. */ >>>>>> && index_sval->get_type () >>>>>> - && TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE >>>>>> + && (!check_index_type >>>>>> + || TREE_CODE (TREE_TYPE (index)) == ENUMERAL_TYPE) >>>>>> && TREE_CODE (index_sval->get_type ()) == ENUMERAL_TYPE >>>>>> /* If we have a constant, then we can check it directly. */ >>>>>> && index_sval->get_kind () != SK_CONSTANT >>>>>> && edge.implicitly_created_default_p () >>>>>> - && has_nondefault_cases_for_all_enum_values_p (switch_stmt) >>>>>> + && has_nondefault_cases_for_all_enum_values_p (switch_stmt, >>>>>> + index_sval- >>>>>>> get_type ()) >>>>>> /* Don't do this if there's a chance that the index is >>>>>> attacker-controlled. */ >>>>>> && !ctxt->possibly_tainted_p (index_sval)) >>>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>>> new file mode 100644 >>>>>> index 0000000000000..98f6d91f97481 >>>>>> --- /dev/null >>>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-no-short-enum-1.c >>>>>> @@ -0,0 +1,141 @@ >>>>>> +/* { dg-do compile } */ >>>>>> +/* { dg-additional-options "-fno-short-enums" } */ >>>>>> +/* { dg-skip-if "default" { ! short_enums } } */ >>>>>> + >>>>>> +#include "analyzer-decls.h" >>>>>> + >>>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>>> + >>>>>> +enum e >>>>>> +{ >>>>>> + E_VAL0, >>>>>> + E_VAL1, >>>>>> + E_VAL2 >>>>>> +}; >>>>>> + >>>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>>> implicit >>>>>> + "default" if all enum values have cases */ >>>>>> + >>>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + case E_VAL2: >>>>>> + return 1945; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> +} >>>>>> + >>>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>>> +{ >>>>>> + int result; >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + result = 1066; >>>>>> + break; >>>>>> + case E_VAL1: >>>>>> + result = 1776; >>>>>> + break; >>>>>> + case E_VAL2: >>>>>> + result = 1945; >>>>>> + break; >>>>>> + } >>>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>>> +} >>>>>> + >>>>>> +/* Verify that we consider paths that use the implicit default when >>>>>> not >>>>>> + all enum values are covered by cases. */ >>>>>> + >>>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>>> +{ >>>>>> + int result; >>>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + result = 1066; >>>>>> + break; >>>>>> + case E_VAL1: >>>>>> + result = 1776; >>>>>> + break; >>>>>> + } >>>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>>> +} >>>>>> + >>>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>>> + >>>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + case E_VAL2: >>>>>> + return 1945; >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 0; >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + default: >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 1945; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_just_default (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 42; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> diff --git a/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>>> b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>>> new file mode 100644 >>>>>> index 0000000000000..384113fde5cbf >>>>>> --- /dev/null >>>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/switch-short-enum-1.c >>>>>> @@ -0,0 +1,140 @@ >>>>>> +/* { dg-do compile } */ >>>>>> +/* { dg-additional-options "-fshort-enums" } */ >>>>>> +/* { dg-skip-if "default" { short_enums } } */ >>>>>> + >>>>>> +#include "analyzer-decls.h" >>>>>> + >>>>>> +/* Verify the handling of "switch (enum_value)". */ >>>>>> + >>>>>> +enum e >>>>>> +{ >>>>>> + E_VAL0, >>>>>> + E_VAL1, >>>>>> + E_VAL2 >>>>>> +}; >>>>>> + >>>>>> +/* Verify that we assume that "switch (enum)" doesn't follow >>>>>> implicit >>>>>> + "default" if all enum values have cases */ >>>>>> + >>>>>> +int test_all_values_covered_implicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + case E_VAL2: >>>>>> + return 1945; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> +} >>>>>> + >>>>>> +int test_all_values_covered_implicit_default_2 (enum e x) >>>>>> +{ >>>>>> + int result; >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + result = 1066; >>>>>> + break; >>>>>> + case E_VAL1: >>>>>> + result = 1776; >>>>>> + break; >>>>>> + case E_VAL2: >>>>>> + result = 1945; >>>>>> + break; >>>>>> + } >>>>>> + return result; /* { dg-bogus "uninitialized" } */ >>>>>> +} >>>>>> + >>>>>> +/* Verify that we consider paths that use the implicit default when >>>>>> not >>>>>> + all enum values are covered by cases. */ >>>>>> + >>>>>> +int test_missing_values_implicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_implicit_default_2 (enum e x) >>>>>> +{ >>>>>> + int result; >>>>>> + switch (x) /* { dg-message "following 'default:' branch" } */ >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + result = 1066; >>>>>> + break; >>>>>> + case E_VAL1: >>>>>> + result = 1776; >>>>>> + break; >>>>>> + } >>>>>> + return result; /* { dg-warning "uninitialized" } */ >>>>>> +} >>>>>> + >>>>>> +/* Verify that explicit "default" isn't rejected. */ >>>>>> + >>>>>> +int test_all_values_covered_explicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + case E_VAL2: >>>>>> + return 1945; >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 0; >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_explicit_default_1 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + default: >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_missing_values_explicit_default_2 (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + case E_VAL0: >>>>>> + return 1066; >>>>>> + case E_VAL1: >>>>>> + return 1776; >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 1945; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +int test_just_default (enum e x) >>>>>> +{ >>>>>> + switch (x) >>>>>> + { >>>>>> + default: >>>>>> + __analyzer_dump_path (); /* { dg-message "path" } */ >>>>>> + return 42; >>>>>> + } >>>>>> + __analyzer_dump_path (); /* { dg-bogus "path" } */ >>>>>> + return 0; >>>>>> +} >>>>>> >>>>>> >>>>> ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-04-10 13:24 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-11-19 7:39 [PATCH] analyzer: deal with -fshort-enums Alexandre Oliva 2023-11-20 22:32 ` David Malcolm 2023-11-22 7:58 ` Alexandre Oliva 2023-12-06 5:31 ` Alexandre Oliva 2023-12-06 22:22 ` David Malcolm 2024-02-07 16:21 ` Torbjorn SVENSSON 2024-02-22 8:51 ` [PING] " Torbjorn SVENSSON 2024-03-08 9:14 ` [PING^2] " Torbjorn SVENSSON 2024-03-15 10:32 ` [PING^3] " Torbjorn SVENSSON 2024-03-25 14:59 ` [PING^4] " Yvan ROUX - foss 2024-04-10 13:23 ` [PING^5] " Torbjorn SVENSSON
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).