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 0AAB93858D37 for ; Thu, 29 Jun 2023 19:00:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0AAB93858D37 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-3fbc54caad5so1367635e9.2 for ; Thu, 29 Jun 2023 12:00:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1688065232; x=1690657232; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=w9Jtx0bqa0nyfyjX5w8/9+0nsFrgfgNVkyzPX9zHBS4=; b=Uk8SL09nRhkQBzMOgM+r3kTUTAJQ1D8iYGw04LbYyh5krJg9OJvD+DWhMzL3C6JJ1y snluJ+PLGsKwdajWmb//iknUC/UUqeLP+FGY43+z9DCc2BPnGbcGzS9a7uzrIq1Dk0LT CKb1QZ9S+0M74TdSovjT/G2IjQ+6l8HI2sNnF4Z5W+oQk1xexpSJnksu5GOJ4k5aWdLw ZfuQlN7r4PdjYkUMwCYCTtsXTzfCSfFsgMM+X0UpBy4/z1DIRqDT9ywqsHnNf6K6MPsW XE/Y6PHWvlszWOE/7XqDafeqcfTIT/fI0bT4NNmu4zV2ryJYeMpytesOBcOj7d2s1ALm eeaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1688065232; x=1690657232; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=w9Jtx0bqa0nyfyjX5w8/9+0nsFrgfgNVkyzPX9zHBS4=; b=HHPm2RKksfJXQ+jDbxpz4E/5IxdO0FYmfG1tlO5W2aR79no3FTTmOuV8VGkiTbuNX1 0ZFF0ej0TUT3Fk0zSRvrmNK/b14TREZmhbflfGLiLASY4rYilDkGF8w3F9/rRndpTAqU 9vKyCxuXk+zEOLlF2s9XW4GTTc9QK4Tt7BTJJZ75MFVEu+T55nKMDntZliLB9tcTIbnA DmwHA+J/QK3lUqbDvRIr2OWvGrWuBV79WbtsET4AOhkMbOE2/rsxQRcX1sdGPXFfLDmT g4PpY+OD3SLA70g03EehjKPkcOTV723SBFDMzVOY0ni6OE8cE25NMFwDT//yfyVxvUN3 Hb9g== X-Gm-Message-State: AC+VfDzxL+h4eQ55xxMbd0elxg1lmHvqKZneFISPKHWUr3asr6r4W9+0 VKK1IlcYI1Ut7+epfci/5HkfuR78oJsv X-Google-Smtp-Source: APBJJlFLFyNyfplOkoZCbNpPMdQuJMlc5Z0E2X3i8OADNkFIosfy2prAtA2dogEDcTFpkzw6DMQqyA== X-Received: by 2002:a5d:6106:0:b0:311:19f9:5824 with SMTP id v6-20020a5d6106000000b0031119f95824mr316103wrt.55.1688065211552; Thu, 29 Jun 2023 12:00:11 -0700 (PDT) Received: from localhost ([2a01:e0a:2ec:f0d0:d98e:5043:2097:b270]) by smtp.gmail.com with UTF8SMTPSA id e15-20020adfe7cf000000b0030aed4223e0sm16440647wrn.105.2023.06.29.12.00.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 29 Jun 2023 12:00:11 -0700 (PDT) From: priour.be@gmail.com X-Google-Original-From: vultkayn@gcc.gnu.org To: gcc-patches@gcc.gnu.org Cc: benjamin priour , dmalcolm@redhat.com Subject: Re: [PATCH] analyzer: Fix regression bug after r14-1632-g9589a46ddadc8b [PR110198] Date: Thu, 29 Jun 2023 20:45:15 +0200 Message-Id: <20230629184510.750143-1-vultkayn@gcc.gnu.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,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: From: benjamin priour See below formatting updates on my patch. In mail https://gcc.gnu.org/pipermail/gcc-patches/2023-June/623140.html, David Malcolm says regtesting failed for him. So I did it once more this morning rebased on fresh trunk dc93a0f633b and target x86_64-linux-gnu, the output was similar to last time: # from gcc_sources_testing gcc/contrib/compare_tests ../gcc_sources_control/build build # Comparing directories ## Dir1=../gcc_sources_control/build: 8 sum files ## Dir2=build: 8 sum files # Comparing 8 common sum files ## /bin/sh gcc/contrib/compare_tests /tmp/gxx-sum1.750468 /tmp/gxx-sum2.750468 Tests that now work, but didn't before (3 tests): g++.dg/analyzer/pr100244.C -std=c++14 (test for warnings, line 17) g++.dg/analyzer/pr100244.C -std=c++17 (test for warnings, line 17) g++.dg/analyzer/pr100244.C -std=c++20 (test for warnings, line 17) # No differences found in 8 common sum files Can you confirm formatting of the patch below, and perhaps regtest it ? Thanks a lot, as this regression fix is now long due. 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