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 AM David Malcolm <dmalc...@redhat.com> > > 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=format-security -Wp,- > > > D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions - > > > fstack- > > > protector-strong -grecord-gcc-switches -m64 -mtune=generic - > > > fasynchronous-unwind-tables -fstack-clash-protection -fcf- > > > protection - > > > D_GNU_SOURCE -fPIC -fwrapv -DDYNAMIC_ANNOTATIONS_ENABLED=1 - > > > DNDEBUG - > > > O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 > > > - > > > Wp,- > > > D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong - > > > grecord- > > > gcc-switches -m64 -mtune=generic -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 == 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!). > > 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 PM David Malcolm > > > > <dmalc...@redhat.com> > > > > 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 — 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. > > > > > > > > > > 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 > > > > > > > > > > > > > > >