From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) by sourceware.org (Postfix) with ESMTPS id 53DD83858D35 for ; Fri, 8 Mar 2024 09:15:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 53DD83858D35 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 53DD83858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=91.207.212.93 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709889322; cv=none; b=W9tdCp/sYo8yVfExcqVdtIBXrXq9bWuVDJxuG12O+EL39vVHc1zgZwaVQHKJrq5CNcuXyYWduLcLcA7MFoDk9LGS8mcvEqg1j3W7x8IBiRXfI3PxvPu9OX7wJ/0qUp2xHBCBfJY7zpDqhotnLqg/lPK99uqX4IkGCjmZy4/Ua1Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709889322; c=relaxed/simple; bh=GWIZOR+FiYTx7VIAyObp1Nk6oBZwvXxcPcM4N2lll2E=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:From:To; b=AFJPh1oCkpiC5AiV3j0Uv85Ui7WrIaDCmMOArUeuuLYpqrJPjk5jMVf+tytAb4wgKFjCVzbQTSb/yYFDwSpNzrSAnHI7fkZ0qakpwTgK8zDRNCjvn8Z1Ige2PjiNW6zznEGx1+/J4tfFl2jXaYvzqd/YJ11CVHAHIUdDHR8EtEc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0046660.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 42883VIj021289; Fri, 8 Mar 2024 10:15:16 +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=K3re5Ab1tGqTRx2IyvBKUr2c6P9iQ736c28iBymoFlQ=; b=Va IihylcnX31qHJiM2EgyEMUfoV2nlDtzyETnmrNiftY8R9Lc6fusbhSLlcLDdiBpX 35x71D2KXFL4LMa2HTkDu/bv/fUHx+Spp+KZMZCZkqP0ny22YQ6ZQpg5x6eCNNiV ImU3StuLHaCgeD4IZsLxsM3Z3xl0B+YW+EjjTQ3WEvmYWZOzVBWtz83cFNvgLaHm tfkYBZcBxTTd49bSjwdDgs7xWm089VsZea87rjlNm6e+Z4LzH/Peua1JKk62JIlM WaM1oaocaiBIlOjddhXVWaFfHJHtrgHb4y7ofhfb/e/hwjhlghUpphsRtYQfTR6F 4Sm4zOsKoRZ/Kap8W2/g== 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 3wqdxmm553-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 08 Mar 2024 10:15:16 +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 950B340044; Fri, 8 Mar 2024 10:15:12 +0100 (CET) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 12897255E7E; Fri, 8 Mar 2024 10:14:59 +0100 (CET) Received: from [10.252.28.121] (10.252.28.121) 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, 8 Mar 2024 10:14:58 +0100 Message-ID: <8a06a0b3-5e56-4ff8-9d3c-f5ca5260e6ae@foss.st.com> Date: Fri, 8 Mar 2024 10:14:57 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PING^2] 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> Content-Language: en-US In-Reply-To: <923c3d78-6d0b-482d-9fcf-3a0129ad7062@foss.st.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.252.28.121] X-ClientProxiedBy: SHFCAS1NODE2.st.com (10.75.129.73) 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-08_06,2024-03-06_01,2023-05-22_02 X-Spam-Status: No, score=-9.9 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-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; >>>> +} >>>> >>>> >>>