This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][test] Use python specified by build rather than system default python
ClosedPublic

Authored by leonardchan on Sep 30 2021, 3:14 PM.

Details

Summary

As of e9564c3698edffc64439a8f957c7c28b19214613, libcxx/gdb/gdb_pretty_printer_test.sh.cpp fails locally for me because the REQUIRES check for host-has-gdb-with-python uses python, which for me expands to python 2.7.18. This failure does not seem to be caught on any upstream builders, potentially because they don't have gdb, python, or a version of python that makes the test UNSUPPORTED (like python3).

This updates the check to use the python specified by the build (which should be the python that runs this code), rather than just python.

Diff Detail

Event Timeline

leonardchan created this revision.Sep 30 2021, 3:14 PM
leonardchan requested review of this revision.Sep 30 2021, 3:14 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 30 2021, 3:14 PM
phosek accepted this revision.Sep 30 2021, 3:17 PM

LGTM

This revision was not accepted when it landed; it landed in state Needs Review.Sep 30 2021, 3:35 PM
This revision was automatically updated to reflect the committed changes.

First off I appreciate you taking the time to look into the detail and I will revert the enabling of the u16 tests while I continue to investigate.

But I think you've got the wrong end of the stick with this change. Wall of text follows because I was confused too so I went to confirm.

Otherwise, I think it may be better to revert because this could potentially fail on any working builders that happen to have gdb.

A bit of background, I'm trying to update our bots to a newer OS and this test fails there because gdb is finally new enough to run it. I've not been able to find another bot that actually runs the pretty printer tests at all. So enabling the u16 tests was partly to flush out anyone else that was having the same issues. (again I'll revert that, it shouldn't be your burden to deal with, thanks for reporting!)

This updates the check to use the python specified by the build (which should be the python that runs this code), rather than just python.

python is a built in command in gdb not a way to run python as you do from the shell.

(gdb) help python
Evaluate a Python command.

The command can be given as an argument, for instance:

    python print 23

If no argument is given, the following lines are read and used
as the Python commands.  Type a line containing "end" to indicate
the end of the command.

This is python on my test system:

$ python
Python 2.7.17 (default, Feb 27 2021, 15:10:58)

The version of python that gdb uses depends on what it was linked to.

$ ldd $(which gdb)
<...>
        libpython3.6m.so.1.0 => /usr/lib/aarch64-linux-gnu/libpython3.6m.so.1.0 (0x0000ffffb6a3a000)
<...>

$ gdb -ex "python import sys; print(sys.version)" --batch
3.6.9 (default, Jan 26 2021, 15:33:00)
$ ldd /home/david.spickett/gdb-8.3/gdb/gdb
<...>
        libpython2.7.so.1.0 => /usr/lib/aarch64-linux-gnu/libpython2.7.so.1.0 (0x0000ffff96328000)
<...>

$ /home/david.spickett/gdb-8.3/gdb/gdb --data-directory=/home/david.spickett/gdb-8.3/gdb/data-directory/ -ex "python import sys; print sys.version" --batch
2.7.17 (default, Feb 27 2021, 15:10:58)

And just for completeness, you can see that passing the path of python doesn't work. Well, gdb returns 0 status but it's a silent failure which, confusingly means that this feature check actually passes.

$ which python3
/usr/bin/python3
$ gdb -ex "/usr/bin/python3 print("foo")" --batch
Undefined command: "".  Try "help".

I also checked that a source-ed file uses the linked python version:

$ cat /tmp/test.py
from __future__ import print_function
import sys; print(sys.version)

$ gdb -ex "source /tmp/test.py" --batch
3.6.9 (default, Jan 26 2021, 15:33:00)

$ /home/david.spickett/gdb-8.3/gdb/gdb --data-directory=/home/david.spickett/gdb-8.3/gdb/data-directory/ -ex "source /tmp/test.py" --batch
2.7.17 (default, Feb 27 2021, 15:10:58)

So the sequence of events and python versions when you run the test suite goes something like this:

  • ninja invokes lit
  • lit uses whatever python cmake chose
  • that python version is used to invoke gdb as a subprocess which uses it's *linked* python to run the provided feature test code
  • (assuming that worked...)
  • lit finds gdb_pretty_printer_test.sh.cpp
  • a RUN command invokes gdb
  • gdb loads the pretty printers using the python it is *linked* to
  • gdb sources gdb_pretty_printer_test.py which runs using the *linked* python version

Meaning that the feature test code and the test itself run on the same version of python.

So this change doesn't really do anything but make the gdb feature test pass all the time (unless sys.executable happens to literally be python). Since gdb will return status 0 and we won't see "Python not supported". So I will revert this along with the u16 change.

In general, there is definitely something odd with the unicode parts of the pretty printer tests. For us at Linaro it's the u32 and u16 tests for you it seems to be just the u16 which is interesting in itself.

Could you tell me your:

  • OS/architecture
  • gdb version
  • python version that gdb is linked to

Then I can at least see if that fixes the u32 tests on our side.

So this change doesn't really do anything but make the gdb feature test pass all the time (unless sys.executable happens to literally be python). Since gdb will return status 0 and we won't see "Python not supported".

Since the test itself doesn't check the same thing the feature test does. The idea is that the feature test checks up front what the test needs because if we fail during the test script, the way it's written, gdb will still exit with status 0.

Indeed, the test is showing in the latest buildkite run as being run but not failing and I know for a fact that our existing arm bots' version of gdb is too old to properly run it. Previously it was marked unsupported.

Oh, thanks for catching a bunch of things I missed and reverting (both the u16 test and this patch). I completely missed that python in this case was a gdb command here. Thanks for the explanation also.

Could you tell me your:

  • OS/architecture

x86_64-unknown-linux-gnu

  • gdb version

GNU gdb (GDB) 10.0-gg5

  • python version that gdb is linked to

I think my libpython appears to be statically linked in(?)

$ ldd $(which gdb)
	linux-vdso.so.1 (0x00007ffe6a340000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa2a7b13000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa2a7af1000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fa2a7aeb000)
	libutil.so.1 => /lib/x86_64-linux-gnu/libutil.so.1 (0x00007fa2a7ae6000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa2a7921000)
	/usr/grte/v4/lib64/ld-linux-x86-64.so.2 => /lib64/ld-linux-x86-64.so.2 (0x00007fa2a7c82000)

But I think I should be able to check with:

$ gdb --ex "python import sys; print sys.version" --batch
2.7.6 (default, Mar 30 2016, 12:21:08) 
[GCC 4.8.x-google 20131105 (prerelease)]

Thanks for the info, I'll see if I can reproduce your results.