From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-00364e01.pphosted.com (mx0b-00364e01.pphosted.com [148.163.139.74]) by sourceware.org (Postfix) with ESMTPS id CFD2D3858D28 for ; Mon, 7 Aug 2023 18:31:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CFD2D3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=columbia.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=columbia.edu Received: from pps.filterd (m0167077.ppops.net [127.0.0.1]) by mx0b-00364e01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 377H51t7022868 for ; Mon, 7 Aug 2023 14:31:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=columbia.edu; h=mime-version : references : in-reply-to : from : date : message-id : subject : to : cc : content-type : content-transfer-encoding; s=pps01; bh=LXcT6bVLzCmugXn7tpAiLHIGYga2YSpzMz8BTLmYmQ8=; b=LlgGNVGJMqqw+DP2groF4sSJpgZjExkJn5j2EisBcPeqfCFCElJOmWjNSTyg8AlqWpMg LGO8Mn9HQZ9GU7JuJY4eHw8icYSjB+UHFCX5Qbs8/6mUOymcHhFDMSwGXjik3scoQQBl 9Qzpguv4iefqz3WbsIXNCOucycFALOp6D5MWpmz0Sgb2F1G2IkXRXs8zVRBfsB6Z++Pl 1nbCoehbJxjTh8hRqC4fjJoa8SRTmJmXYFIzd/gncKCPtX0G8eXDWnSeq6D/FXzBO8Rs +zXZ+1ZywBtO3zmidCMSuShNd1VvaitsGayGwIxkfGA1gwt1gYzw8yXuqMTf7NgTE+wG dw== Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) by mx0b-00364e01.pphosted.com (PPS) with ESMTPS id 3s9g0v4tbr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Mon, 07 Aug 2023 14:31:20 -0400 Received: by mail-ua1-f71.google.com with SMTP id a1e0cc1a2514c-78a5bbabf67so967473241.0 for ; Mon, 07 Aug 2023 11:31:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691433080; x=1692037880; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=LXcT6bVLzCmugXn7tpAiLHIGYga2YSpzMz8BTLmYmQ8=; b=eCJuFsCcsbxLnINP2IzA0FQ1nwFGXVFViKASVC6/j6c4BSlH2AE+9IQ88vLxPXGJko 8YnKTkNEQ61j90Wr0XFfH9317+mDhPtwpc8GQI5zdckcM4H9u27Q7dVt325P3M9PwrPn z5k9HhoO95Rd5CQkka6uRc81hi0fY1oElFkaUFgE6C4aD5lgKWbhl8yMn1vjPGN+4pMf M8YzSaV1AAAk5n2wEpL63ei0gbgrUF81fTRyBJVnyF/yf4QmSy7SBJ0AjELvW91iAuZI ybZjohADwEcZ13Ey0mJRFQAY7EyQ4U2g8FSbPFSF+57jsB+QvVbnGaT7G4RB/oascAFP qPNA== X-Gm-Message-State: AOJu0YzjPdvA6VN6Bym53O6tlETDs3dKXuWUCa3eFBiVJey7b6QLvMIL n4aVMpdzaPrf8Ll2w9Cl97IEMlot0MFl6WjRT5MdGDI6mstZjUavYEM0gEuylThewhpEE+ck3mK v6ae54F6mSx9y0bqgSvgfplk+X110cck= X-Received: by 2002:a1f:3f54:0:b0:487:4939:e8c0 with SMTP id m81-20020a1f3f54000000b004874939e8c0mr2246378vka.13.1691433079778; Mon, 07 Aug 2023 11:31:19 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHYmwD5t97Dnv4e8qpOiGzEvt+8iozFDRVZQI8a5Q1fuAsV2ik7wVH5DqrZZy1OQvKL7bbWgZcrZ2L6PoshYM8= X-Received: by 2002:a1f:3f54:0:b0:487:4939:e8c0 with SMTP id m81-20020a1f3f54000000b004874939e8c0mr2246363vka.13.1691433079409; Mon, 07 Aug 2023 11:31:19 -0700 (PDT) MIME-Version: 1.0 References: <969057b59e5cf472b73e8e1dedcc4a46630b31a0.camel@redhat.com> <6050a30719fed76a5bdb36c00620f4cb44e00aad.camel@redhat.com> <38828e8aa3d0905c2c006ff635534953e88ad91f.camel@redhat.com> <54a16f618fe937382481250853807bedfbcb1a62.camel@redhat.com> <1c5026021761abd92ded93950e31e4ff77ca250a.camel@redhat.com> In-Reply-To: <1c5026021761abd92ded93950e31e4ff77ca250a.camel@redhat.com> From: Eric Feng Date: Mon, 7 Aug 2023 14:31:08 -0400 Message-ID: Subject: Re: Update and Questions on CPython Extension Module -fanalyzer plugin development To: David Malcolm Cc: gcc@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Proofpoint-GUID: WR-IvJGg-b3ejP6OSq2LkOj6BbEAXy4X X-Proofpoint-ORIG-GUID: WR-IvJGg-b3ejP6OSq2LkOj6BbEAXy4X X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-08-07_20,2023-08-03_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 clxscore=1015 impostorscore=10 suspectscore=0 mlxlogscore=999 malwarescore=0 spamscore=0 priorityscore=1501 lowpriorityscore=10 bulkscore=10 phishscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000 definitions=main-2308070169 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 List-Id: On Fri, Aug 4, 2023 at 6:46=E2=80=AFPM David Malcolm = wrote: > > On Fri, 2023-08-04 at 18:42 -0400, David Malcolm wrote: > > On Fri, 2023-08-04 at 16:48 -0400, Eric Feng wrote: > > > On Fri, Aug 4, 2023 at 11:39=E2=80=AFAM David Malcolm > > > wrote: > > > > > > > > On Fri, 2023-08-04 at 11:02 -0400, Eric Feng wrote: > > > > > Hi Dave, > > > > > > > > > > Tests related to our plugin which depend on Python-specific > > > > > definitions have been run by including /* { dg-options "- > > > > > fanalyzer > > > > > -I/usr/include/python3.9" } */. This is undoubtedly not ideal; > > > > > is > > > > > it > > > > > best to approach this problem by adapting a subset of relevant > > > > > definitions like in gil.h? > > > > > > > > That might be acceptable in the very short-term, but to create a > > > > plugin > > > > that's useful to end-user (authors of CPython extension modules) > > > > we > > > > want to be testing against real Python headers. > > > > > > > > As I understand it, https://peps.python.org/pep-0394/ allows for > > > > distributors of Python to symlink "python3-config" in the PATH to > > > > a > > > > python3.X-config script (for some X). > > > > > > > > So on such systems running: > > > > python3-config --includes > > > > should emit the correct -I option. On my box it emits: > > > > > > > > -I/usr/include/python3.8 -I/usr/include/python3.8 > > > > > > > > > > > > It's more complicated, but I believe: > > > > python3-config --cflags > > > > should emit the build flags that C/C++ extensions ought to use > > > > when > > > > building. On my box this emits: > > > > > > > > -I/usr/include/python3.8 -I/usr/include/python3.8 -Wno-unused- > > > > result - > > > > Wsign-compare -O2 -g -pipe -Wall -Werror=3Dformat-security -Wp,- > > > > D_FORTIFY_SOURCE=3D2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions - > > > > fstack- > > > > protector-strong -grecord-gcc-switches -m64 -mtune=3Dgeneric - > > > > fasynchronous-unwind-tables -fstack-clash-protection -fcf- > > > > protection - > > > > D_GNU_SOURCE -fPIC -fwrapv -DDYNAMIC_ANNOTATIONS_ENABLED=3D1 - > > > > DNDEBUG - > > > > O2 -g -pipe -Wall -Werror=3Dformat-security -Wp,-D_FORTIFY_SOURCE= =3D2 > > > > - > > > > Wp,- > > > > D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong - > > > > grecord- > > > > gcc-switches -m64 -mtune=3Dgeneric -fasynchronous-unwind-tables - > > > > fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC - > > > > fwrapv > > > > > > > > and it's likely going to vary from distribution to distribution. > > > > Some > > > > of those options *are* going to affect the gimple that -fanalyzer > > > > "sees". > > > > > > > > Does your installation of Python have such a script? > > > > > > > > So in the short term you could hack in a minimal subset of the > > > > decls/defns from Python.h, but I'd prefer it if target- > > > > supports.exp > > > > gained a DejaGnu directive that invokes python3-config, captures > > > > the > > > > result (or fails with UNSUPPORTED for systems without python3 > > > > development headers), and then adds the result to the build flags > > > > of > > > > the file being tested. The .exp files are implemented in Tcl, > > > > alas; > > > > let me know if you want help with that. > > > > > > > > Dave > > > Sounds good; thanks! Following existing examples in > > > target-supports.exp, the following works as expected in terms of > > > extracting the build flags we are interested in. > > > > > > In target-supports.exp: > > > proc check_python_flags { } { > > > set result [remote_exec host "python3-config --cflags"] > > > set status [lindex $result 0] > > > if { $status =3D=3D 0 } { > > > return [lindex $result 1] > > > } else { > > > return "UNSUPPORTED" > > > } > > > } > > > > > > However, I'm having some trouble figuring out the specifics as to > > > how > > > we may add the build flags to our test cases. My intuition looks > > > like > > > something like the following: > > > > > > In plugin.exp: > > > foreach plugin_test $plugin_test_list { > > > if {[lindex $plugin_test 0] eq "analyzer_cpython_plugin.c"} { > > > set python_flags [check_python_flags] > > > if { $python_flags ne "UNSUPPORTED" } { > > > // append $python_flags to build flags here > > > } > > > } > > > .... > > > } > > > > > > How might we do so? > > > > Good question. > > > > Looking at plugin.exp I see it uses plugin-test-execute, which is > > defined in gcc/testsuite/lib/plugin-support.exp. > > > > Looking there, I see it attempts to build the plugin, and then if it > > succeeds, it calls > > dg-runtest $plugin_tests $plugin_enabling_flags $default_flags > > where $plugin_tests is the list of source files to be compiled using > > the plugin. So one way to do this would be to modify that code from > > plugin.exp to pass in a different value, rather than $default_flags. > > Though it seems hackish to special-case this. > > Sorry, I think I misspoke here; that line that uses $default_flags is > from plugin-support.exp, not from plugin.exp; $default_flags is a > global variable. > > So I think my 2nd approach below may be the one to try: > > > > > As another way, that avoids adding special-casing to plugin.exp, > > there's an existing directive: > > dg-additional-options > > implemented in gcc/testsuite/lib/gcc-defs.exp which appends options > > to > > the default options. Unfortunately, it works via: > > upvar dg-extra-tool-flags extra-tool-flags > > which is a nasty Tcl hack meaning access the local variable named > > "dg- > > extra-tool-flags" in *the frame above*, referring to it as "extra- > > tool- > > flags". (this is why I don't like Tcl) > > > > So I think what could be done is to invoke your "check_python_flags" > > test as a directive from the test case, so that in target- > > supports.exp > > you'd have something like: > > > > proc dg-require-python-h {} { > > > > which could do the invocation/output-capture of python3-config, and > > would also have code similar to that in dg-additional-options to > > append > > to the options (or it could possibly just call dg-additional-options > > provided there's an "upvar" before the callsite to make the nested > > stack manipulation work). > > > > The individual test cases could then have: > > > > /* { dg-require-python-h } */ > > > > in them. > > > > That way the Tcl stack at the point where the new directive runs > > should > > be similar enough to how dg-additional-options gets run for similar > > option-injection code to work (yuck!). Gotcha, thanks for the tip! I've been trying to make this approach work, but despite trying many things just having /* { dg-require-python-h } */ in the relevant test cases does not seem to invoke proc dg-require-python-h {} { ... } in target-supports.exp. Am I missing something else? Do we need to also "register" it somewhere else for it to recognize the command? For context, previously I was able to see the results of "check_python_flags" (i.e the output of python3-config) by invoking it in plugin.exp. > > > > Maybe someone else on the list can see a less hackish way to get this > > to work? > > > > Let me know if any of the above is unclear. > > Dave > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Best, > > > > > Eric > > > > > > > > > > On Tue, Aug 1, 2023 at 1:06=E2=80=AFPM David Malcolm > > > > > > > > > > wrote: > > > > > > > > > > > > On Tue, 2023-08-01 at 09:57 -0400, Eric Feng wrote: > > > > > > > > > > > > > > > > My guess is that you were trying to do it from the > > > > > > > > PLUGIN_ANALYZER_INIT > > > > > > > > hook rather than from the plugin_init function, but it's > > > > > > > > hard > > > > > > > > to be > > > > > > > > sure without seeing the code. > > > > > > > > > > > > > > > > > > > > > > Thanks Dave, you are entirely right =E2=80=94 I made the mist= ake of > > > > > > > trying to > > > > > > > do it from PLUGIN_ANALYZER_INIT hook and not from the > > > > > > > plugin_init > > > > > > > function. After following your suggestion, the callbacks > > > > > > > are > > > > > > > getting > > > > > > > registered as expected. > > > > > > > > > > > > Ah, good. > > > > > > > > > > > > > I submitted a patch to review for this feature > > > > > > > on gcc-patches; please let me know if it looks OK. > > > > > > > > > > > > Thanks Eric; I've posted a reply to your email there, so > > > > > > let's > > > > > > discuss > > > > > > the details there. > > > > > > > > > > > > Dave > > > > > > > > > > > > > > > > > > > > >