From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A91093858C2B for ; Fri, 1 Sep 2023 14:51:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A91093858C2B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1693579865; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LV9Xfh4bSbqMw0Ranrb+FE2Xo9R7NEWvmOJHlWVjEuM=; b=WDNRqn9Bm0VL7sypStAt04xnqGTq/K+LBY0p5VkdeYB2UM+rxdHZQtRdLEr6I5VplO4JV5 dGNUujcZwZHv/UC/O3a7oHQt5qbeTqqY6QG6EokVro2QaER0q/V29DFCMLB+V6wKfxjdU1 tYqeeLyWUG7IQhGorYACIZ1uYB3kljg= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-447-S1mA-QGkOvuZCQeVbLuONA-1; Fri, 01 Sep 2023 10:51:03 -0400 X-MC-Unique: S1mA-QGkOvuZCQeVbLuONA-1 Received: by mail-qv1-f70.google.com with SMTP id 6a1803df08f44-64d09183380so20181346d6.0 for ; Fri, 01 Sep 2023 07:51:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693579863; x=1694184663; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=LV9Xfh4bSbqMw0Ranrb+FE2Xo9R7NEWvmOJHlWVjEuM=; b=Y+ysJ2gnb1kYgun+HljMy3fSQCF0G5GSoC8ibjNY+AhQoPYPxj9YPJIgScYTY2hodX 6KbjPCNV0O7GjWgy649JmhLZE5q+GVhQfuLbnEW/VC/3Z6sI8Ok6mgNT26oXnRjpr39n lnke1uvF+llo+J0+Dpvw3l+ybVn3+jEUy3CuX8vzwEK3G4n2e13+qlTQzulacxc9bzTt bMlYSMdRL5ojOdFprvztN3HxPF5ZNiKH8ZA42BwgZ9opXms+Ju9JI0Z5HVAOhUUUnFaL CM+L6LT10LGFWmah7wk+awWn6vQf3/aSHQYdhvfLtAFJxpF/ifeFt7NQVHVnqH9sXn6B YgwQ== X-Gm-Message-State: AOJu0Yz+xEWCOK/Y9owx/Xm+AtfAA+hIHHbYQ3h1Ax4+RxaYftnYkWsM lWIrevd7lf/sPSJbpndgDwxy3QAa0xDrnlBHxXLTrgzxvb5wXuh4bc1ItkbFHR1OCp/0gDeoiSV yLQHUsjw= X-Received: by 2002:a0c:b31e:0:b0:64f:4253:f3ef with SMTP id s30-20020a0cb31e000000b0064f4253f3efmr2563662qve.15.1693579863256; Fri, 01 Sep 2023 07:51:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG8yMhyxuFYjwNt0LAub5RzE5Cko+VfoSPjOwDhjumRCTFcBmVGm06Xr5S7GSpKxaa8kKIA0Q== X-Received: by 2002:a0c:b31e:0:b0:64f:4253:f3ef with SMTP id s30-20020a0cb31e000000b0064f4253f3efmr2563646qve.15.1693579862948; Fri, 01 Sep 2023 07:51:02 -0700 (PDT) Received: from t14s.localdomain (c-76-28-97-5.hsd1.ma.comcast.net. [76.28.97.5]) by smtp.gmail.com with ESMTPSA id z14-20020a0cda8e000000b0063cfe9adb2csm1463957qvj.108.2023.09.01.07.51.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Sep 2023 07:51:02 -0700 (PDT) Message-ID: <456ca70cfade6081f770310423e4e8926c10dd77.camel@redhat.com> Subject: Re: [PATCH] analyzer: implement reference count checking for CPython plugin [PR107646] From: David Malcolm To: Hans-Peter Nilsson , Eric Feng Cc: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org Date: Fri, 01 Sep 2023 10:51:00 -0400 In-Reply-To: <20230901024948.8496820427@pchp3.se.axis.com> References: <20230829043155.17651-1-ef2648@columbia.edu> <20230901024948.8496820427@pchp3.se.axis.com> User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00,BODY_8BITS,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,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 List-Id: On Fri, 2023-09-01 at 04:49 +0200, Hans-Peter Nilsson wrote: > (Looks like this was committed as r14-3580-g597b9ec69bca8a) >=20 > > Cc: gcc@gcc.gnu.org, gcc-patches@gcc.gnu.org, Eric Feng > > > > From: Eric Feng via Gcc >=20 > > gcc/testsuite/ChangeLog: > > =C2=A0 PR analyzer/107646 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/analyze= r_cpython_plugin.c: Implements > > reference count > > =C2=A0 * checking for PyObjects. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-2.c: Moved to... > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-PyList_Append.c: > > ...here (and > > =C2=A0 * added more tests). > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-1.c: Moved to... > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-no-plugin.c: ...here > > (and added > > =C2=A0 * more tests). > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/plugin.= exp: New tests. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-PyList_New.c: New test. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-PyLong_FromLong.c: New > > test. > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/cpython= -plugin-test-refcnt-checking.c: New > > test. >=20 > It seems this was more or less a rewrite, but that said, > it's generally preferable to always *add* tests, never *modify* them. >=20 > > =C2=A0.../gcc.dg/plugin/analyzer_cpython_plugin.c=C2=A0=C2=A0 | 376 > > +++++++++++++++++- >=20 > ^^^ Ouch!=C2=A0 Was it not within reason to keep that test as it > was, and just add another test? >=20 > Anyway, the test after rewrite fails, and for some targets > like cris-elf and apparently m68k-linux, yields an error. > I see a PR was already opened. >=20 > Also, mostly for future reference, several files in the > patch miss a final newline, as seen by a "\ No newline at > end of file"-marker. >=20 > I think I found the problem; a mismatch between default C++ > language standard between host-gcc and target-gcc. >=20 > (It's actually *not* as simple as "auto var =3D typeofvar()" > not being recognized in C++11 --or else there'd be an error > for the hash_set declaration too, which I just changed for > consistency-- but it's close enough for me.) >=20 > With this, retesting plugin.exp for cris-elf works. >=20 > Ok to commit? Sorry about the failing tests. Thanks for the patch; please go ahead and commit. Dave >=20 > -- >8 -- > From: Hans-Peter Nilsson > Date: Fri, 1 Sep 2023 04:36:03 +0200 > Subject: [PATCH] testsuite: Fix analyzer_cpython_plugin.c > declarations, PR testsuite/111264 >=20 > Also, add missing newline at end of file. >=20 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0PR testsuite/111264 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.dg/plugin/analyzer_= cpython_plugin.c: Make declarations > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0C++11-compatible. > --- > =C2=A0gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c | 8 ++++---- > =C2=A01 file changed, 4 insertions(+), 4 deletions(-) >=20 > diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c > b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c > index 7af520436549..bf1982e79c37 100644 > --- a/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c > +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_cpython_plugin.c > @@ -477,8 +477,8 @@ pyobj_refcnt_checker (const region_model *model, > =C2=A0=C2=A0 if (!ctxt) > =C2=A0=C2=A0=C2=A0=C2=A0 return; > =C2=A0 > -=C2=A0 auto region_to_refcnt =3D hash_map (); > -=C2=A0 auto seen_regions =3D hash_set (); > +=C2=A0 hash_map region_to_refcnt; > +=C2=A0 hash_set seen_regions; > =C2=A0 > =C2=A0=C2=A0 count_pyobj_references (model, region_to_refcnt, retval, > seen_regions); > =C2=A0=C2=A0 check_refcnts (model, old_model, retval, ctxt, region_to_ref= cnt); > @@ -561,7 +561,7 @@ public: > =C2=A0=C2=A0=C2=A0=C2=A0 if (!ctxt) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return; > =C2=A0=C2=A0=C2=A0=C2=A0 region_model *model =3D cd.get_model (); > -=C2=A0=C2=A0=C2=A0 auto region_to_refcnt =3D hash_map (); > +=C2=A0=C2=A0=C2=A0 hash_map region_to_refcnt; > =C2=A0=C2=A0=C2=A0=C2=A0 count_all_references(model, region_to_refcnt); > =C2=A0=C2=A0=C2=A0=C2=A0 dump_refcnt_info(region_to_refcnt, model, ctxt); > =C2=A0=C2=A0 } > @@ -1330,4 +1330,4 @@ plugin_init (struct plugin_name_args > *plugin_info, > =C2=A0=C2=A0 sorry_no_analyzer (); > =C2=A0#endif > =C2=A0=C2=A0 return 0; > -} > \ No newline at end of file > +}