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 CBD3F3858D1E for ; Fri, 4 Aug 2023 22:46:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CBD3F3858D1E 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=1691189187; 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=gcB++T0INxRRx9YtoUMfpENqgki9LJ8Fdm3SiyseiYo=; b=hf4aBB8eIrov8iTib4PpGewPoqH+CG3l70QpkYBtZXhPDZUjl45Ty8CepF8BhbBv4qyz98 NU1+mUTGvhMsKAWA+eDVRk0NB9licYzfaUrOKlRw+Jyq6WBQfs2Q/6sQtoh88wIayvIiy8 wAsPWObgUuKeV/ZHx+GYzC8nLxvJUv8= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-642-M-YWhMABOaiBAfGmVz5l_A-1; Fri, 04 Aug 2023 18:46:26 -0400 X-MC-Unique: M-YWhMABOaiBAfGmVz5l_A-1 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7675583f2d0so326652385a.2 for ; Fri, 04 Aug 2023 15:46:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691189185; x=1691793985; 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=gcB++T0INxRRx9YtoUMfpENqgki9LJ8Fdm3SiyseiYo=; b=NG5KWKByywk+gYIaupiGXL/AbOiEVSl+ATYwGNRbM1oym5FC+V5MaT2b2u0pl1SbhG KtkFxcRdxIJeHxMHx5WTXAk3JTUEzaDbw00LsAy1cgOB15j3Y4PcHFL9cBjf6e3nB10L MKfPdZ09+DZ5m7dyuzpiqh2fpyRImmY8M4soYRJF0dHAcC9nby54dCnzwj1o10s6u1xZ VljLTb69tr1sduDZ21Z1wCqAWkxfeappTctVUloxDP1OXZUjVHBPgluy1L2lYzEJLGs3 Wc9yJvqJ/0yV5WwgXu89NjMpQ7QDszblCy70wdKgpwfJgGX9sgdQuLvY/fUgqy0wJgj2 v6yg== X-Gm-Message-State: AOJu0YzFz6+Z5SjwOqsVEgK0Wfm17MsnAEdsZu0Fa+2pXzBEnjL03yL+ C2RrtgcOtMG5zmNKw5tbyKb8I+pL6outeJFgXuLdWqARGI6xEglBVV12KsBZZefoQ4Kz7V8OMcI tYsHVRO73cSbWmE8= X-Received: by 2002:a05:622a:10e:b0:407:613a:c383 with SMTP id u14-20020a05622a010e00b00407613ac383mr3896276qtw.67.1691189185631; Fri, 04 Aug 2023 15:46:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHEat/tJ+vz/UO+lJDLIm6b3JIHvAKEYnu8ac0F/D8k80fgdmLmzovgvD24vjSMLNKwvODCRA== X-Received: by 2002:a05:622a:10e:b0:407:613a:c383 with SMTP id u14-20020a05622a010e00b00407613ac383mr3896254qtw.67.1691189185265; Fri, 04 Aug 2023 15:46:25 -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 ka4-20020a05622a440400b00405447ee5e8sm973108qtb.55.2023.08.04.15.46.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Aug 2023 15:46:24 -0700 (PDT) Message-ID: <1c5026021761abd92ded93950e31e4ff77ca250a.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: Fri, 04 Aug 2023 18:46:24 -0400 In-Reply-To: <54a16f618fe937382481250853807bedfbcb1a62.camel@redhat.com> References: <969057b59e5cf472b73e8e1dedcc4a46630b31a0.camel@redhat.com> <6050a30719fed76a5bdb36c00620f4cb44e00aad.camel@redhat.com> <38828e8aa3d0905c2c006ff635534953e88ad91f.camel@redhat.com> <54a16f618fe937382481250853807bedfbcb1a62.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 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=3Dgene= ric - > > > fasynchronous-unwind-tables -fstack-clash-protection -fcf- > > > protection - > > > D_GNU_SOURCE -fPIC -fwrapv=C2=A0 -DDYNAMIC_ANNOTATIONS_ENABLED=3D1 - > > > 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-unwind-= tables - > > > fstack-clash-protection -fcf-protection -D_GNU_SOURCE -fPIC - > > > fwrapv > > >=20 > > > and it's likely going to vary from distribution to distribution.=C2= =A0 > > > 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 --cflag= s"] > > =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_plu= gin.c"} { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 set python_flags [check_pyth= on_flags] > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if { $python_flags ne "UNSUP= PORTED" } { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 // append = $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=20 > =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.=20 > 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: >=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!). >=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 mistak= e 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