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.133.124]) by sourceware.org (Postfix) with ESMTPS id 904373858C41 for ; Mon, 7 Aug 2023 23:16:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 904373858C41 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=1691450165; 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=hW6LBmsFNi5nxBzQ/B7Rg92dIPRztu1k+Kc1WZPuI50=; b=OwvVrbXlDcyC5x5G1KIYPf5ATg7YyIKHub4F+canzrZvXQpnQQmk2mYXxlGhJ9/4KUOV0E SyoJMkZHIRQOhn3iJnRyCHJ8XiEmm9uj1ZOa2t194FL3SNZNWPD6XRUcXf+EMtCW1lRqMb k0OA6peRVmBA06FbVM9M2Uzag2/kM/w= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-10-C08xwpo8PIWyEtGHToQFfQ-1; Mon, 07 Aug 2023 19:16:04 -0400 X-MC-Unique: C08xwpo8PIWyEtGHToQFfQ-1 Received: by mail-qk1-f198.google.com with SMTP id af79cd13be357-76ce6d007ccso348435985a.1 for ; Mon, 07 Aug 2023 16:16:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691450163; x=1692054963; 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=hW6LBmsFNi5nxBzQ/B7Rg92dIPRztu1k+Kc1WZPuI50=; b=WCX+qUqJ4+xtbdzIA/hSYT18jrVSmUw1QpNfDGfiXBXqzQ/kJOv7ixZR76p3WjMbHx 60bvNndvo5ULW9kvjckEANfzY2GKc+jG5Oy4/V7FiEjyM3XiWhHc5tvBJ/166+9QCnYP 4dAyDE0D9AB7qYL91lH/CMG0Jb/PowGA5Pcf6DEcIY3QEYRO6WKBn0Xp63clYkTYDdBP 27SSOmzp3C+FJMwJCS+FMCdCL9vCSUglXsG2QvFSxta+UF4J1qZAQmlvsJRah4LuUk88 ho4FRAyjUHIrzqrc0n47wPr/dbhttKpGEKNpg4DLHPjFtNQIdIV6DJft4UkpLvmDZjH/ h5Cw== X-Gm-Message-State: AOJu0YzfmNlj4RTctLcJLyUN9SdDhUDjH+G6rgBxuRUfTBMLGO2+cir/ QGpU+BAWsU5G2IbDzJ0L/2DdNGWRYW8WrB6JdprU2lXRP7RS7xVZJrgkEzSTngUn81EtcuKSsQC 8BESb+E7hHQOAjvw= X-Received: by 2002:a05:620a:c4d:b0:765:758d:5be8 with SMTP id u13-20020a05620a0c4d00b00765758d5be8mr8087450qki.0.1691450163248; Mon, 07 Aug 2023 16:16:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGxA0eqfsAjXmNxV2raATo7EwnXFh9D3kkE7+YN/ir703LQtEoJLPRnf5svFR84TlNIjFdlKg== X-Received: by 2002:a05:620a:c4d:b0:765:758d:5be8 with SMTP id u13-20020a05620a0c4d00b00765758d5be8mr8087434qki.0.1691450162890; Mon, 07 Aug 2023 16:16: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 m16-20020ae9e010000000b0076ccf1a0da3sm2929532qkk.75.2023.08.07.16.16.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Aug 2023 16:16:02 -0700 (PDT) Message-ID: <925a3480a840b43634c303d508ec2f908c92edff.camel@redhat.com> Subject: Re: Update and Questions on CPython Extension Module -fanalyzer plugin development From: David Malcolm To: Eric Feng Cc: gcc@gcc.gnu.org Date: Mon, 07 Aug 2023 19:16:01 -0400 In-Reply-To: References: <969057b59e5cf472b73e8e1dedcc4a46630b31a0.camel@redhat.com> <6050a30719fed76a5bdb36c00620f4cb44e00aad.camel@redhat.com> <38828e8aa3d0905c2c006ff635534953e88ad91f.camel@redhat.com> <54a16f618fe937382481250853807bedfbcb1a62.camel@redhat.com> <1c5026021761abd92ded93950e31e4ff77ca250a.camel@redhat.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=-3.7 required=5.0 tests=BAYES_00,BODY_8BITS,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 Mon, 2023-08-07 at 14:31 -0400, Eric Feng wrote: > On Fri, Aug 4, 2023 at 6:46=E2=80=AFPM David Malcolm > wrote: > >=20 > > 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: > > > > >=20 > > > > > On Fri, 2023-08-04 at 11:02 -0400, Eric Feng wrote: > > > > > > Hi Dave, > > > > > >=20 > > > > > > 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? > > > > >=20 > > > > > 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. > > > > >=20 > > > > > As I understand it, https://peps.python.org/pep-0394/=C2=A0allows > > > > > for > > > > > distributors of Python to symlink "python3-config" in the > > > > > PATH to > > > > > a > > > > > python3.X-config script (for some X). > > > > >=20 > > > > > So on such systems running: > > > > > =C2=A0 python3-config --includes > > > > > should emit the correct -I option.=C2=A0 On my box it emits: > > > > >=20 > > > > > -I/usr/include/python3.8 -I/usr/include/python3.8 > > > > >=20 > > > > >=20 > > > > > It's more complicated, but I believe: > > > > > =C2=A0 python3-config --cflags > > > > > should emit the build flags that C/C++ extensions ought to > > > > > use > > > > > when > > > > > building.=C2=A0 On my box this emits: > > > > >=20 > > > > > -I/usr/include/python3.8 -I/usr/include/python3.8=C2=A0 -Wno- > > > > > unused- > > > > > result - > > > > > Wsign-compare=C2=A0 -O2 -g -pipe -Wall -Werror=3Dformat-security = - > > > > > Wp,- > > > > > D_FORTIFY_SOURCE=3D2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions - > > > > > fstack- > > > > > protector-strong -grecord-gcc-switches=C2=A0=C2=A0 -m64 -mtune=3D= generic > > > > > - > > > > > fasynchronous-unwind-tables -fstack-clash-protection -fcf- > > > > > protection - > > > > > D_GNU_SOURCE -fPIC -fwrapv=C2=A0 -DDYNAMIC_ANNOTATIONS_ENABLED=3D= 1 - > > > > > DNDEBUG=C2=A0 - > > > > > O2 -g -pipe -Wall -Werror=3Dformat-security -Wp,- > > > > > D_FORTIFY_SOURCE=3D2 > > > > > - > > > > > Wp,- > > > > > D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong - > > > > > grecord- > > > > > gcc-switches=C2=A0=C2=A0 -m64 -mtune=3Dgeneric -fasynchronous-unw= ind- > > > > > tables - > > > > > fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC - > > > > > fwrapv > > > > >=20 > > > > > and it's likely going to vary from distribution to > > > > > distribution. > > > > > Some > > > > > of those options *are* going to affect the gimple that - > > > > > fanalyzer > > > > > "sees". > > > > >=20 > > > > > Does your installation of Python have such a script? > > > > >=20 > > > > > 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.=C2=A0 The .exp files are implemented in > > > > > Tcl, > > > > > alas; > > > > > let me know if you want help with that. > > > > >=20 > > > > > 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. > > > >=20 > > > > In target-supports.exp: > > > > proc check_python_flags { } { > > > > =C2=A0=C2=A0=C2=A0 set result [remote_exec host "python3-config --c= flags"] > > > > =C2=A0=C2=A0=C2=A0 set status [lindex $result 0] > > > > =C2=A0=C2=A0=C2=A0 if { $status =3D=3D 0 } { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return [lindex $result 1= ] > > > > =C2=A0=C2=A0=C2=A0 } else { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return "UNSUPPORTED" > > > > =C2=A0=C2=A0=C2=A0 } > > > > } > > > >=20 > > > > 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: > > > >=20 > > > > In plugin.exp: > > > > foreach plugin_test $plugin_test_list { > > > > =C2=A0=C2=A0=C2=A0 if {[lindex $plugin_test 0] eq "analyzer_cpython= _plugin.c"} > > > > { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 set python_flags [check_= python_flags] > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if { $python_flags ne "U= NSUPPORTED" } { > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // app= end $python_flags to build flags here > > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > > > =C2=A0=C2=A0=C2=A0 } > > > > .... > > > > } > > > >=20 > > > > How might we do so? > > >=20 > > > Good question. > > >=20 > > > Looking at plugin.exp I see it uses plugin-test-execute, which is > > > defined in gcc/testsuite/lib/plugin-support.exp. > > >=20 > > > Looking there, I see it attempts to build the plugin, and then if > > > it > > > succeeds, it calls > > > =C2=A0 dg-runtest $plugin_tests $plugin_enabling_flags $default_flags > > > where $plugin_tests is the list of source files to be compiled > > > using > > > the plugin.=C2=A0 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. > >=20 > > 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. > >=20 > > So I think my 2nd approach below may be the one to try: > >=20 > > >=20 > > > As another way, that avoids adding special-casing to plugin.exp, > > > there's an existing directive: > > > =C2=A0=C2=A0 dg-additional-options > > > implemented in gcc/testsuite/lib/gcc-defs.exp which appends > > > options > > > to > > > the default options.=C2=A0 Unfortunately, it works via: > > > =C2=A0=C2=A0=C2=A0 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".=C2=A0 (this is why I don't like Tcl) > > >=20 > > > 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: > > >=20 > > > =C2=A0 proc dg-require-python-h {} { > > >=20 > > > 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). > > >=20 > > > The individual test cases could then have: > > >=20 > > > =C2=A0 /* { dg-require-python-h } */ > > >=20 > > > in them. > > >=20 > > > 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. My understanding is that ultimately we call into DejGnu's dg.exp's dg- get-options, which has this grep: set tmp [grep $prog "{\[ \t\]\+dg-\[-a-z\]\+\[ \t\]\+.*\[ \t\]\+}" line= ] to find directives in the source file, the first part of which are function names. plugin.exp has a load_lib of target-supports.exp, so it *should* know about the dg-require-python-h function. Have a look in the .log for lines that say "ERROR", which can appear if the syntax of a directive is wrong. Failing that, please post a patch with the work-in-progress version of the various parts of this that you're using, so I can see exactly what you're doing, and I can have a go at debugging it. Thanks Dave > > >=20 > > > Maybe someone else on the list can see a less hackish way to get > > > this > > > to work? > > >=20 > > > Let me know if any of the above is unclear. > > > Dave > > >=20 > > >=20 > > >=20 > > >=20 > > >=20 > > >=20 > > > > >=20 > > > > >=20 > > > > > >=20 > > > > > > Best, > > > > > > Eric > > > > > >=20 > > > > > > On Tue, Aug 1, 2023 at 1:06=E2=80=AFPM David Malcolm > > > > > > > > > > > > wrote: > > > > > > >=20 > > > > > > > On Tue, 2023-08-01 at 09:57 -0400, Eric Feng wrote: > > > > > > > > >=20 > > > > > > > > > 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. > > > > > > > > >=20 > > > > > > > >=20 > > > > > > > > Thanks Dave, you are entirely right =E2=80=94 I made the > > > > > > > > mistake 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. > > > > > > >=20 > > > > > > > Ah, good. > > > > > > >=20 > > > > > > > > I submitted a patch to review for this feature > > > > > > > > on gcc-patches; please let me know if it looks OK. > > > > > > >=20 > > > > > > > Thanks Eric; I've posted a reply to your email there, so > > > > > > > let's > > > > > > > discuss > > > > > > > the details there. > > > > > > >=20 > > > > > > > Dave > > > > > > >=20 > > > > > >=20 > > > > >=20 > > > >=20 > > >=20 > >=20 >=20