Re: [PATCH] analyzer: deal with -fshort-enums

[Alexandre Oliva <oliva@adacore.com>]

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