From patchwork Tue Aug 29 17:28:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Feng X-Patchwork-Id: 1827381 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=rc5rMS3G; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4RZvY75Yjqz1yfX for ; Wed, 30 Aug 2023 03:28:58 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 83EF73857C44 for ; Tue, 29 Aug 2023 17:28:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 83EF73857C44 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1693330135; bh=gHMcY89klSEBIzTgjiye1hVmuXzEtU8gtrwIn1VTT3w=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=rc5rMS3GHDocBSLqj5L0UyKPWOVhgfav9tzNYSPOmvBEWCfYNefcxTp55gHPimfT8 Qk5PpGPSNF7rrk6a/Y0i1Ay4EMTh7dCoPiByvlf6/7xETih6Kgq7eXw1b5Z+xnYyOE S1MEFMyoVhghtPOvunIf8lo2nPD0+9LuktBUcaOs= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0b-00364e01.pphosted.com (mx0b-00364e01.pphosted.com [148.163.139.74]) by sourceware.org (Postfix) with ESMTPS id 0987C385840F for ; Tue, 29 Aug 2023 17:28:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0987C385840F Received: from pps.filterd (m0167074.ppops.net [127.0.0.1]) by mx0b-00364e01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 37TGPQ9S011306 for ; Tue, 29 Aug 2023 13:28:31 -0400 Received: from mail-vk1-f197.google.com (mail-vk1-f197.google.com [209.85.221.197]) by mx0b-00364e01.pphosted.com (PPS) with ESMTPS id 3sr58qrrwu-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Tue, 29 Aug 2023 13:28:31 -0400 Received: by mail-vk1-f197.google.com with SMTP id 71dfb90a1353d-49047d42d9eso963932e0c.3 for ; Tue, 29 Aug 2023 10:28:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693330110; x=1693934910; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gHMcY89klSEBIzTgjiye1hVmuXzEtU8gtrwIn1VTT3w=; b=gIxwAQzUOhXW5laAEg72Ee2OgN1ANiFIhNxex0j+8kd1wHc9U9y/Nip0imTspSAa/z MNWBaIZeG+VWJq7wsJk9R1pPMEapU2pXhf0BPeflXQqCrGfXstNroJ/leDT9UAqw8s2z 6gMoDBmemZshKILnHTsXphuK+wk6N92kToHw2m+tYFEgF6mJRXPHX2MZT2pQT8ZUi42F EqT3ND91XOI64Pzr+4cZtTVt1Xi6QohmXcXLQ+4q+e7FAxKP5gNKn4w9tIVmYi4Y9ZZG hrQdlxVLSahOHFuaa04f0xye5UWGzhuL7JhTSFtV0aGydr0vZs+vZBolVypAJ0BP2TJz EgIA== X-Gm-Message-State: AOJu0YyrJR9ceQKzOIEeokoPupCA/A8twetLSnG6GZFqPM+mo2tiQtEo zXn/jGltD5LPIkdB5JBARnlmYmpnj0XT2LKVL/pU/c0CrC6EGXTQv22fYZKrGBukjVIskYOJED0 MPQ58FrGyebK4BmY= X-Received: by 2002:a1f:728e:0:b0:48d:eaa:45c4 with SMTP id n136-20020a1f728e000000b0048d0eaa45c4mr25821670vkc.7.1693330110393; Tue, 29 Aug 2023 10:28:30 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG07a3UHojUP/oKLq0QOMGFU+iDPAOSOyBF2MT8g5NMAPYymc+lPpBUBa4GZH2Spmi5EIHDMg== X-Received: by 2002:a1f:728e:0:b0:48d:eaa:45c4 with SMTP id n136-20020a1f728e000000b0048d0eaa45c4mr25821650vkc.7.1693330110043; Tue, 29 Aug 2023 10:28:30 -0700 (PDT) Received: from Ericcs-MBP.cable.rcn.com (207-38-164-63.s9254.c3-0.43d-cbr1.qens-43d.ny.cable.rcncustomer.com. [207.38.164.63]) by smtp.gmail.com with ESMTPSA id i20-20020a0cf394000000b005dd8b9345b4sm3473867qvk.76.2023.08.29.10.28.29 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Tue, 29 Aug 2023 10:28:29 -0700 (PDT) To: dmalcolm@redhat.com Cc: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org, Eric Feng Subject: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646] Date: Tue, 29 Aug 2023 13:28:18 -0400 Message-Id: <20230829172818.3264-1-ef2648@columbia.edu> X-Mailer: git-send-email 2.32.0 (Apple Git-132) In-Reply-To: References: MIME-Version: 1.0 X-Proofpoint-ORIG-GUID: GuXfoRvEjbYksbw1IOK4eGRYeld7WWd6 X-Proofpoint-GUID: GuXfoRvEjbYksbw1IOK4eGRYeld7WWd6 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.601,FMLib:17.11.176.26 definitions=2023-08-29_13,2023-08-29_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 mlxscore=0 lowpriorityscore=10 adultscore=0 malwarescore=0 suspectscore=0 impostorscore=10 priorityscore=1501 bulkscore=10 mlxlogscore=999 spamscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2308100000 definitions=main-2308290151 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Eric Feng via Gcc-patches From: Eric Feng Reply-To: Eric Feng Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Additionally, by using the old model and the pointer per your suggestion, we are able to find the representative tree and emit a more accurate diagnostic! rc3.c:23:10: warning: expected ‘item’ to have reference count: ‘1’ but ob_refcnt field is: ‘2’ 23 | return list; | ^~~~ ‘create_py_object’: events 1-4 | | 4 | PyObject* item = PyLong_FromLong(3); | | ^~~~~~~~~~~~~~~~~~ | | | | | (1) when ‘PyLong_FromLong’ succeeds | 5 | PyObject* list = PyList_New(1); | | ~~~~~~~~~~~~~ | | | | | (2) when ‘PyList_New’ succeeds |...... | 14 | PyList_Append(list, item); | | ~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (3) when ‘PyList_Append’ succeeds, moving buffer |...... | 23 | return list; | | ~~~~ | | | | | (4) here | If a representative tree is not found, I decided we should just bail out of emitting a diagnostic for now, to avoid confusing the user on what the problem is. I've attached the patch for this (on top of the previous one) below. If it also looks good, I can merge it with the last patch and push it in at the same time. Best, Eric --- gcc/analyzer/region-model.cc | 3 +- gcc/analyzer/region-model.h | 7 ++-- .../gcc.dg/plugin/analyzer_cpython_plugin.c | 35 +++++++++++-------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index eb4f976b83a..c1d266d351b 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -5391,6 +5391,7 @@ region_model::pop_frame (tree result_lvalue, { gcc_assert (m_current_frame); + const region_model pre_popped_model = *this; const frame_region *frame_reg = m_current_frame; /* Notify state machines. */ @@ -5424,7 +5425,7 @@ region_model::pop_frame (tree result_lvalue, } unbind_region_and_descendents (frame_reg,POISON_KIND_POPPED_STACK); - notify_on_pop_frame (this, retval, ctxt); + notify_on_pop_frame (this, &pre_popped_model, retval, ctxt); } /* Get the number of frames in this region_model's stack. */ diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 440ea6d828d..b89c6f6c649 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -237,6 +237,7 @@ public: struct append_regions_cb_data; typedef void (*pop_frame_callback) (const region_model *model, + const region_model *prev_model, const svalue *retval, region_model_context *ctxt); @@ -543,11 +544,13 @@ class region_model } static void - notify_on_pop_frame (const region_model *model, const svalue *retval, + notify_on_pop_frame (const region_model *model, + const region_model *prev_model, + const svalue *retval, region_model_context *ctxt) { for (auto &callback : pop_frame_callbacks) - callback (model, retval, ctxt); + callback (model, prev_model, retval, ctxt); } private: diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c index b2caed8fc1b..6f0a355fe30 100644 --- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c @@ -318,11 +318,10 @@ public: auto actual_refcnt = m_actual_refcnt->dyn_cast_constant_svalue ()->get_constant (); auto ob_refcnt = m_ob_refcnt->dyn_cast_constant_svalue ()->get_constant (); - warned = warning_meta ( - rich_loc, m, get_controlling_option (), - "expected to have " - "reference count: %qE but ob_refcnt field is: %qE", - actual_refcnt, ob_refcnt); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "expected %qE to have " + "reference count: %qE but ob_refcnt field is: %qE", + m_reg_tree, actual_refcnt, ob_refcnt); // location_t loc = rich_loc->get_loc (); // foo (loc); @@ -425,7 +424,8 @@ count_expected_pyobj_references (const region_model *model, /* Compare ob_refcnt field vs the actual reference count of a region */ static void -check_refcnt (const region_model *model, region_model_context *ctxt, +check_refcnt (const region_model *model, const region_model *old_model, + region_model_context *ctxt, const hash_map::iterator::reference_pair region_refcnt) { @@ -438,8 +438,11 @@ check_refcnt (const region_model *model, region_model_context *ctxt, if (ob_refcnt_sval != actual_refcnt_sval) { - // todo: fix this - tree reg_tree = model->get_representative_tree (curr_region); + const svalue *curr_reg_sval + = mgr->get_ptr_svalue (pyobj_ptr_tree, curr_region); + tree reg_tree = old_model->get_representative_tree (curr_reg_sval); + if (!reg_tree) + return; const auto &eg = ctxt->get_eg (); refcnt_stmt_finder finder (*eg, reg_tree); @@ -451,20 +454,22 @@ check_refcnt (const region_model *model, region_model_context *ctxt, } static void -check_refcnts (const region_model *model, const svalue *retval, - region_model_context *ctxt, - hash_map ®ion_to_refcnt) +check_refcnts (const region_model *model, const region_model *old_model, + const svalue *retval, region_model_context *ctxt, + hash_map ®ion_to_refcnt) { for (const auto ®ion_refcnt : region_to_refcnt) { - check_refcnt(model, ctxt, region_refcnt); + check_refcnt(model, old_model, ctxt, region_refcnt); } } /* Validates the reference count of all Python objects. */ void -pyobj_refcnt_checker (const region_model *model, const svalue *retval, - region_model_context *ctxt) +pyobj_refcnt_checker (const region_model *model, + const region_model *old_model, + const svalue *retval, + region_model_context *ctxt) { if (!ctxt) return; @@ -473,7 +478,7 @@ pyobj_refcnt_checker (const region_model *model, const svalue *retval, auto seen_regions = hash_set (); count_expected_pyobj_references (model, region_to_refcnt, retval, seen_regions); - check_refcnts (model, retval, ctxt, region_to_refcnt); + check_refcnts (model, old_model, retval, ctxt, region_to_refcnt); } /* Counts the actual pyobject references from all clusters in the model's