From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x334.google.com (mail-wm1-x334.google.com [IPv6:2a00:1450:4864:20::334]) by sourceware.org (Postfix) with ESMTPS id C11AD3858D39 for ; Wed, 28 Jun 2023 11:11:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C11AD3858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-x334.google.com with SMTP id 5b1f17b1804b1-3fa99b57a38so32498165e9.0 for ; Wed, 28 Jun 2023 04:11:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687950660; x=1690542660; h=in-reply-to:content-language:references:cc:to:subject:from :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=h25tsC2k51XVW5Ezn3XA2pB2nn90mdgcmHl/fkKpPkM=; b=h6SkkOm0AzFdaBM2TlfpJZHMrdBrFIaOY4+KzvO79r+gVa2d7xL14WfzpzQYQU5H4w LSVmMypC+oS2aA64YpDQls6NXlpWl7yW9zct1+ohuXsNHqQYGkV6miIZmbsXYf7qru/g xwAppuUOSoiA9gV1xBzx466IZC+0rbvAbY0wp/e5FeFQBklXDyuBZsGJLFeYkBQEaHtE PN/edFyP+uQjcS2Bef/dn2bOSSCzgoTQyHKQuWXI2TZP9+zSIgPzJJ5u+tHTtN1VtuAD W8oFdHcZ6KtcIg4qyTn5QtRu0PQ7R/Q6y8cIXTSbJbN5GtCzZDjpn4DfAAb6G+M1ix4M A/Mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687950660; x=1690542660; h=in-reply-to:content-language:references:cc:to:subject:from :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=h25tsC2k51XVW5Ezn3XA2pB2nn90mdgcmHl/fkKpPkM=; b=aF1f5h8IydfvdQICa+Zdd9qDVuUvW1KdYRgoo66LA9MTxV46WYv0vEZacbk5QbtVQf mikKTxIaaAHT7342gM+p4Ke221BLrQhFcAdcUF04E7df6TT+yN6MBsyt4qbevE99nRBX +vkg000X2pmR4NWhtzpGT8cAVQYOUjOHakCPPz1zkzGN5+XpcTOj/Pjj5Pm88VCbgYoA LsPtM9ODNMDiRh1Y2OvA8qKwfci6eTVgYyt8vTkjFaNGs9ZjjpBwHrya7e46zRHimt8K PbJXuzDStPis9lxGLN/ai3vPHqeJeu/z0E78cfpJqaPAnwR+f6Mbq/LiazYvBflUINUz d+yA== X-Gm-Message-State: AC+VfDzvutyuBFR1aiBhweGJe1QbX4C4jAnzhQSjdFFM+aMEf9g8PltO m66OvV+4jAgzAlD33R5keJswM7HDJvZG X-Google-Smtp-Source: ACHHUZ4pY4ootCNcEXjTN4WK74zhxMnbxjthqzG/820qwJie2VwSxqTi3bEiT0tCSwGkSTKLSIfCBA== X-Received: by 2002:a05:600c:ad4:b0:3f9:b8b8:20df with SMTP id c20-20020a05600c0ad400b003f9b8b820dfmr16850379wmr.33.1687950660137; Wed, 28 Jun 2023 04:11:00 -0700 (PDT) Received: from ?IPV6:2a01:e0a:2ec:f0d0:c02a:c58:6c71:19b9? ([2a01:e0a:2ec:f0d0:c02a:c58:6c71:19b9]) by smtp.gmail.com with ESMTPSA id m21-20020a7bcb95000000b003faabd8fcb8sm7445286wmi.46.2023.06.28.04.10.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 28 Jun 2023 04:10:59 -0700 (PDT) Content-Type: multipart/alternative; boundary="------------Jp8co1jVW0Nh3HBwRbFKiG26" Message-ID: Date: Wed, 28 Jun 2023 13:10:58 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 From: Benjamin Priour Subject: PING: Re: [PATCH] analyzer: Fix regression bug after r14-1632-g9589a46ddadc8b [pr110198] To: gcc-patches@gcc.gnu.org Cc: David Malcolm References: <20230622195522.1834793-1-vultkayn@gcc.gnu.org> Content-Language: en-US In-Reply-To: <20230622195522.1834793-1-vultkayn@gcc.gnu.org> X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,HTML_MESSAGE,KAM_SHORT,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: This is a multi-part message in MIME format. --------------Jp8co1jVW0Nh3HBwRbFKiG26 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Pinging that regression fix. Is everything OK for trunk ? Thanks, Benjamin On Thu, Jun 22, 2023 at 9:57 PM wrote: From: benjamin priour Resend with proper subject line ... Hi, Below is the fix to regression bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110198 Was bootstrapped and regtested successfully on x86_64-linux-gnu Considering mishap from last patch, I would appreciate if you could also regtest it, to be sure :) Thanks, Benjamin. g++.dg/analyzer/pr100244.C was failing after a patch of PR109439. The reason was a spurious preemptive return of get_store_value upon out-of-bounds read that was preventing further checks. Now instead, a boolean value check_poisoned goes to false when a OOB is detected, and is later on given to get_or_create_initial_value. gcc/analyzer/ChangeLog:         * region-model-manager.cc (region_model_manager::get_or_create_initial_value): Take an                 optional boolean value to bypass poisoning checks         * region-model-manager.h: Update declaration of the above function.         * region-model.cc (region_model::get_store_value): No longer                 returns on OOB, but rather gives a boolean to get_or_create_initial_value.         (region_model::check_region_access): Update docstring.         (region_model::check_region_for_write): Update docstring. Signed-off-by: benjamin priour ---  gcc/analyzer/region-model-manager.cc |  5 +++--  gcc/analyzer/region-model-manager.h  |  3 ++-  gcc/analyzer/region-model.cc         | 15 ++++++++-------  3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/gcc/analyzer/region-model-manager.cc b/gcc/analyzer/region-model-manager.cc index 1453acf7bc9..4f11ef4bd29 100644 --- a/gcc/analyzer/region-model-manager.cc +++ b/gcc/analyzer/region-model-manager.cc @@ -293,9 +293,10 @@ region_model_manager::create_unique_svalue (tree type)     necessary.  */  const svalue * -region_model_manager::get_or_create_initial_value (const region *reg) +region_model_manager::get_or_create_initial_value (const region *reg, +                                                  bool check_poisoned)  { -  if (!reg->can_have_initial_svalue_p ()) +  if (!reg->can_have_initial_svalue_p () && check_poisoned)      return get_or_create_poisoned_svalue (POISON_KIND_UNINIT,                                           reg->get_type ()); diff --git a/gcc/analyzer/region-model-manager.h b/gcc/analyzer/region-model-manager.h index 3340c3ebd1e..ff5333bf07c 100644 --- a/gcc/analyzer/region-model-manager.h +++ b/gcc/analyzer/region-model-manager.h @@ -49,7 +49,8 @@ public:                                              tree type);    const svalue *get_or_create_poisoned_svalue (enum poison_kind kind,                                                tree type); -  const svalue *get_or_create_initial_value (const region *reg); +  const svalue *get_or_create_initial_value (const region *reg, +                                            bool check_poisoned = true);    const svalue *get_ptr_svalue (tree ptr_type, const region *pointee);    const svalue *get_or_create_unaryop (tree type, enum tree_code op,                                        const svalue *arg); diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 6bc60f89f3d..187013a37cc 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -2373,8 +2373,9 @@ region_model::get_store_value (const region *reg,    if (reg->empty_p ())      return m_mgr->get_or_create_unknown_svalue (reg->get_type ()); +  bool check_poisoned = true;    if (check_region_for_read (reg, ctxt)) -    return m_mgr->get_or_create_unknown_svalue(reg->get_type()); +    check_poisoned = false;    /* Special-case: handle var_decls in the constant pool.  */    if (const decl_region *decl_reg = reg->dyn_cast_decl_region ()) @@ -2427,7 +2428,7 @@ region_model::get_store_value (const region *reg,        == RK_GLOBALS)      return get_initial_value_for_global (reg); -  return m_mgr->get_or_create_initial_value (reg); +  return m_mgr->get_or_create_initial_value (reg, check_poisoned);  }  /* Return false if REG does not exist, true if it may do. @@ -2790,7 +2791,7 @@ region_model::get_string_size (const region *reg) const  /* If CTXT is non-NULL, use it to warn about any problems accessing REG,     using DIR to determine if this access is a read or write. -   Return TRUE if an UNKNOWN_SVALUE needs be created. +   Return TRUE if an OOB access was detected.     If SVAL_HINT is non-NULL, use it as a hint in diagnostics     about the value that would be written to REG.  */ @@ -2804,10 +2805,10 @@ region_model::check_region_access (const region *reg,    if (!ctxt)      return false; -  bool need_unknown_sval = false; +  bool oob_access_detected = false;    check_region_for_taint (reg, dir, ctxt);    if (!check_region_bounds (reg, dir, sval_hint, ctxt)) -    need_unknown_sval = true; +    oob_access_detected = true;    switch (dir)      { @@ -2820,7 +2821,7 @@ region_model::check_region_access (const region *reg,        check_for_writable_region (reg, ctxt);        break;      } -  return need_unknown_sval; +  return oob_access_detected;  }  /* If CTXT is non-NULL, use it to warn about any problems writing to REG.  */ @@ -2834,7 +2835,7 @@ region_model::check_region_for_write (const region *dest_reg,  }  /* If CTXT is non-NULL, use it to warn about any problems reading from REG. -  Returns TRUE if an unknown svalue needs be created.  */ +  Returns TRUE if an OOB read was detected.  */  bool  region_model::check_region_for_read (const region *src_reg, -- 2.34.1 --------------Jp8co1jVW0Nh3HBwRbFKiG26--