From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id 866183858C42 for ; Thu, 22 Feb 2024 08:51:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 866183858C42 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=foss.st.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 866183858C42 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=185.132.182.106 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708591909; cv=none; b=BCVyBOzVSwR72U1tg28WFqYzU6N+C/TVvPNjO99Bz74y3WW2KBB/MTmy2WRR85Cz9m3xjUtP4m/+XEsPHuxxr7hNlh/kbYytDbY6dxq9JwSiSIfvtFA9tOwtEP9vnG05LiHe4NLs6V2wQpCGIkP4YvIFqotkAggl3i61ZrmmA7k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708591909; c=relaxed/simple; bh=ynQaUoAyzoH5AgWDGnh06qQO7TZ1R9Gj1PB+jGixnYk=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=Es4UEJpbaM36JV4fj7hq8Qpd4CSuOj+zIZpoxhq5TS2GoAdwBr0jS8nKR7PALZVzJSICQ0uo6W+A6/Bf3Xk2e92YgaldpiXKFsFIh1XPiS7k6RVhplbyYVHQ09MXiYf7lF67FEe5EnqFD1QphT/qIEvYwDSjjeBwWCCRAAexnuc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0288072.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 41M5efDl027910; Thu, 22 Feb 2024 09:51:44 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h= message-id:date:mime-version:subject:from:to:cc:references :in-reply-to:content-type:content-transfer-encoding; s= selector1; bh=lrWK4/StJqK3oZYtTJSVDYXHXzYna1mZOwJjN6adEIM=; b=yn w48vftIbf7O9K8gUYdZ5CU/DL9TONMOB/clN4mvsLfdm5zC/6bfvwtxp8tbHXSO+ 3pp6vZfZe1sjXsMpLKSghU/bbocIUTE2NE7oRVSlhCI5G+fGtviLfKS7YcbAgyaP uTaOijMHDSYldqmiie/YC4VN0kqqy64lrC0sd1BIkQsDi30Ev4LzpLc6I/5XvQRR +pggb3Xolt41PiXpaqqsPQR9+7MfTIBUiKeoySs9WW3809HbvCFmDKY5kvN8o85X 1b6sHjcPQ4aO3ukjcVrdAUvbXvlJFsjWrBvQz56COLNJN3tmrK0T6CGr6qZrASOx zxojoVsDST/mo0Ux4Eaw== Received: from beta.dmz-ap.st.com (beta.dmz-ap.st.com [138.198.100.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3wd201q97t-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Feb 2024 09:51:44 +0100 (CET) Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-ap.st.com (STMicroelectronics) with ESMTP id B3BEE40047; Thu, 22 Feb 2024 09:51:38 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 283B62967E8; Thu, 22 Feb 2024 09:51:26 +0100 (CET) Received: from [10.74.18.1] (10.74.18.1) by SHFDAG1NODE3.st.com (10.75.129.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Thu, 22 Feb 2024 09:51:25 +0100 Message-ID: <923c3d78-6d0b-482d-9fcf-3a0129ad7062@foss.st.com> Date: Thu, 22 Feb 2024 09:51:25 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PING] Re: [PATCH] analyzer: deal with -fshort-enums Content-Language: en-US From: Torbjorn SVENSSON To: David Malcolm , Alexandre Oliva CC: , Yvan Roux References: In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.74.18.1] X-ClientProxiedBy: EQNCAS1NODE3.st.com (10.75.129.80) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2024-02-22_06,2024-02-22_01,2023-05-22_02 X-Spam-Status: No, score=-9.4 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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 >>> (index_sval); >>> +      if (unaryop->get_op () == NOP_EXPR >>> +         && is_a (unaryop->get_arg ())) >>> +       if (const initial_svalue *initvalop = (as_a >> 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; >>> +} >>> >>> >>