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 3F2053857B86 for ; Fri, 15 Mar 2024 10:33:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3F2053857B86 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 3F2053857B86 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=1710498794; cv=none; b=hfej7cjHcOW5H80PIlffXRRu6FtELoKsM3/ddTfN10u4669oWIR9goHPk4FOUe/37WFxm85oxqhwD+b96rdR1Mx4FxmQibcCVVQgLjcRRvhpMU0qtYu11u4FoQePsmus1rn48M4MI/gqOZSp75jCyOtljtfMS4CF0HH+DzDXDFA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1710498794; c=relaxed/simple; bh=9Pg3TwzqKbd4B/s9tp5DB0wU6fb4nNwxglh0simzN/I=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=SXXB3P/3ciP+S1Jb7LejelewHvimdvNpgO0T7+BJ9WCYXHpkA1lHE07uIgNyZmr+LU01VVR/0pahLQY5OPBriZ1cXO7WJGBzVkdiVYP0rgF1ci+av7liTyYOlV3SfKXToledN9puXVjCxbFMkxl0KUz/UmKcpu3v286LjwwqSzk= 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 42F8YIfY021929; Fri, 15 Mar 2024 11:33:01 +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=OOyAtooJPXvK3vWyAw1uawcVsfPtZ85TQpqJgpv6o90=; b=a2 rAhYFnBXZMc9tOEWWGP7E9lVqWQhfw9txy2fBSJ/rQqHBRrRU30ac+AXtI9pF6zj +3BIbYaxn6ozgwLDsowqJqMM/mTMene9GZH4obYYHV//7nQHO0ZTYA2TxFD3pWHd gS3U+/3EKCekIist+xT0fTwpnKYEirNMPUliiNQXkS7DEy7MSdXpUjMqqjtPyqL4 D8Az1GV01CvcCdI+lTC6HlrrRqB9Hg1NRuUgbxF9GE6Bq3gR3ORuy0RAgmfC1moz IUXjJ4wbEPnooaYASTxgd5evklY7FG7Bp1IsZe9ZXsViKD94mBo6m6ktagph0H5K YYII94+rxsKyRcPhHqGA== 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 3wv9y9t9qc-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 15 Mar 2024 11:33:00 +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 3A81A40049; Fri, 15 Mar 2024 11:32:57 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 4BC662504FA; Fri, 15 Mar 2024 11:32:45 +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; Fri, 15 Mar 2024 11:32:44 +0100 Message-ID: <97e88998-92f9-4c3e-87c6-5d5f99515e36@foss.st.com> Date: Fri, 15 Mar 2024 11:32:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PING^3] Re: [PATCH] analyzer: deal with -fshort-enums From: Torbjorn SVENSSON To: David Malcolm , Alexandre Oliva CC: , Yvan Roux References: <923c3d78-6d0b-482d-9fcf-3a0129ad7062@foss.st.com> <8a06a0b3-5e56-4ff8-9d3c-f5ca5260e6ae@foss.st.com> Content-Language: en-US In-Reply-To: <8a06a0b3-5e56-4ff8-9d3c-f5ca5260e6ae@foss.st.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.74.18.1] X-ClientProxiedBy: SHFCAS1NODE1.st.com (10.75.129.72) 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-03-14_13,2024-03-13_01,2023-05-22_02 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,RCVD_IN_DNSWL_LOW,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-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 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; >>>>> +} >>>>> >>>>> >>>>