Avoid CPython internals like _PyObject_Dump in universal mode?

[mattip]

The use of CPython internals inside HPy code is problematic. Building in the universal mode should probably define the limited API. Otherwise the universal binaries will not really be universal, they may fail on some versions of CPython. For instance this code is problematic, https://github.com/hpyproject/hpy/blob/da2cefc422eb66ced05d6b4c020f0ffb1920781e/hpy/devel/src/runtime/ctx_object.c#L10-L17 and should probably have some kind of guard like

ctx_Dump(HPyContext *ctx, HPy h)
{
#ifdef PYPY_VERSION
#elif defined(GRAAL_VERSION) // is there something like this?
#elif !defined(Py_LIMITED_API)
    // just use _PyObject_Dump for now, but we might want to add more info
    // about the handle itself in the future.
    _PyObject_Dump(_h2py(h));
#endif
}

[hodgestar]

This is the code that implements the context for universal mode on CPython, I think -- i.e. it is never part of any HPy module. If it is somehow ending up in an HPy extension, then that is a mistake we should fix.

[mattip]

I see it when I am running tests on the 0.0.4 tag after adding PyErr_SetFromErrnoWithFilenameObjects yesterday

pypypy -m pytest test/test_00_basic.py -x

``` =================================================================== test session starts ==================================================================== platform linux -- Python 3.8.13[pypy-7.3.10-alpha], pytest-7.1.2, pluggy-1.0.0 rootdir: /home/matti/oss/hpy collected 72 items test/test_00_basic.py ...F ========================================================================= FAILURES ========================================================================= ___________________________________________________________ TestBasic.test_empty_module[cpython] ___________________________________________________________ self = obj = '/tmp/pytest-of-matti/pytest-14/test_empty_module_cpython_0/home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.o' src = '/home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.c', ext = '.c' cc_args = ['-DHPY', '-I/home/matti/oss/hpy/hpy/devel/include', '-I/tmp/pypy3.8/include', '-I/home/matti/oss/pypy/include/pypy3.8', '-c'] extra_postargs = ['-g', '-O0', '-Wfatal-errors', '-Werror'] pp_opts = ['-DHPY', '-I/home/matti/oss/hpy/hpy/devel/include', '-I/tmp/pypy3.8/include', '-I/home/matti/oss/pypy/include/pypy3.8'] def _compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts): compiler_so = self.compiler_so if sys.platform == 'darwin': compiler_so = _osx_support.compiler_fixup(compiler_so, cc_args + extra_postargs) try: self.spawn(compiler_so + cc_args + [src, '-o', obj] + > extra_postargs) ../pypy/lib-python/3/distutils/unixccompiler.py:133: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = , cmd = ['gcc', '-pthread', '-DNDEBUG', '-O2', '-fPIC', '-DHPY', ...] def spawn(self, cmd): > spawn(cmd, dry_run=self.dry_run) ../pypy/lib-python/3/distutils/ccompiler.py:910: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ cmd = ['gcc', '-pthread', '-DNDEBUG', '-O2', '-fPIC', '-DHPY', ...], search_path = 1, verbose = 0, dry_run = 0 def spawn(cmd, search_path=1, verbose=0, dry_run=0): """Run another program, specified as a command list 'cmd', in a new process. 'cmd' is just the argument list for the new process, ie. cmd[0] is the program to run and cmd[1:] are the rest of its arguments. There is no way to run a program with a name different from that of its executable. If 'search_path' is true (the default), the system's executable search path will be used to find the program; otherwise, cmd[0] must be the exact path to the executable. If 'dry_run' is true, the command will not actually be run. Raise DistutilsExecError if running the program fails in any way; just return on success. """ # cmd is documented as a list, but just in case some code passes a tuple # in, protect our %-formatting code against horrible death cmd = list(cmd) if os.name == 'posix': > _spawn_posix(cmd, search_path, dry_run=dry_run) ../pypy/lib-python/3/distutils/spawn.py:36: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ cmd = 'gcc', search_path = 1, verbose = 0, dry_run = 0 def _spawn_posix(cmd, search_path=1, verbose=0, dry_run=0): log.info(' '.join(cmd)) if dry_run: return executable = cmd[0] exec_fn = search_path and os.execvp or os.execv env = None if sys.platform == 'darwin': global _cfg_target, _cfg_target_split if _cfg_target is None: from distutils import sysconfig _cfg_target = sysconfig.get_config_var( 'MACOSX_DEPLOYMENT_TARGET') or '' if _cfg_target: _cfg_target_split = [int(x) for x in _cfg_target.split('.')] if _cfg_target: # ensure that the deployment target of build process is not less # than that used when the interpreter was built. This ensures # extension modules are built with correct compatibility values cur_target = os.environ.get('MACOSX_DEPLOYMENT_TARGET', _cfg_target) if _cfg_target_split > [int(x) for x in cur_target.split('.')]: my_msg = ('$MACOSX_DEPLOYMENT_TARGET mismatch: ' 'now "%s" but "%s" during configure' % (cur_target, _cfg_target)) raise DistutilsPlatformError(my_msg) env = dict(os.environ, MACOSX_DEPLOYMENT_TARGET=cur_target) exec_fn = search_path and os.execvpe or os.execve pid = os.fork() if pid == 0: # in the child try: if env is None: exec_fn(executable, cmd) else: exec_fn(executable, cmd, env) except OSError as e: if not DEBUG: cmd = executable sys.stderr.write("unable to execute %r: %s\n" % (cmd, e.strerror)) os._exit(1) if not DEBUG: cmd = executable sys.stderr.write("unable to execute %r for unknown reasons" % cmd) os._exit(1) else: # in the parent # Loop until the child either exits or is terminated by a signal # (ie. keep waiting if it's merely stopped) while True: try: pid, status = os.waitpid(pid, 0) except OSError as exc: if not DEBUG: cmd = executable raise DistutilsExecError( "command %r failed: %s" % (cmd, exc.args[-1])) if os.WIFSIGNALED(status): if not DEBUG: cmd = executable raise DistutilsExecError( "command %r terminated by signal %d" % (cmd, os.WTERMSIG(status))) elif os.WIFEXITED(status): exit_status = os.WEXITSTATUS(status) if exit_status == 0: return # hey, it succeeded! else: if not DEBUG: cmd = executable raise DistutilsExecError( "command %r failed with exit status %d" > % (cmd, exit_status)) E distutils.errors.DistutilsExecError: command 'gcc' failed with exit status 1 ../pypy/lib-python/3/distutils/spawn.py:159: DistutilsExecError During handling of the above exception, another exception occurred: self = def test_empty_module(self): import sys > mod = self.make_module(""" @INIT """) test/test_00_basic.py:26: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ test/support.py:365: in make_module extra_sources) test/support.py:271: in make_module main_src, ExtensionTemplate, name, extra_sources) test/support.py:255: in compile_module compiler_verbose=self.compiler_verbose) test/support.py:421: in c_compile outputfilename = _build(tmpdir, ext, hpy_devel, hpy_abi, compiler_verbose, debug) test/support.py:460: in _build dist.run_command('build_ext') ../pypy/lib-python/3/distutils/dist.py:985: in run_command cmd_obj.run() /tmp/pypy3.8/lib/pypy3.8/site-packages/setuptools/command/build_ext.py:79: in run _build_ext.run(self) ../pypy/lib-python/3/distutils/command/build_ext.py:343: in run self.build_extensions() ../pypy/lib-python/3/distutils/command/build_ext.py:452: in build_extensions self._build_extensions_serial() ../pypy/lib-python/3/distutils/command/build_ext.py:477: in _build_extensions_serial self.build_extension(ext) /tmp/pypy3.8/lib/pypy3.8/site-packages/setuptools/command/build_ext.py:202: in build_extension _build_ext.build_extension(self, ext) ../pypy/lib-python/3/distutils/command/build_ext.py:537: in build_extension depends=ext.depends) ../pypy/lib-python/3/distutils/ccompiler.py:574: in compile self._compile(obj, src, ext, cc_args, extra_postargs, pp_opts) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = obj = '/tmp/pytest-of-matti/pytest-14/test_empty_module_cpython_0/home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.o' src = '/home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.c', ext = '.c' cc_args = ['-DHPY', '-I/home/matti/oss/hpy/hpy/devel/include', '-I/tmp/pypy3.8/include', '-I/home/matti/oss/pypy/include/pypy3.8', '-c'] extra_postargs = ['-g', '-O0', '-Wfatal-errors', '-Werror'] pp_opts = ['-DHPY', '-I/home/matti/oss/hpy/hpy/devel/include', '-I/tmp/pypy3.8/include', '-I/home/matti/oss/pypy/include/pypy3.8'] def _compile(self, obj, src, ext, cc_args, extra_postargs, pp_opts): compiler_so = self.compiler_so if sys.platform == 'darwin': compiler_so = _osx_support.compiler_fixup(compiler_so, cc_args + extra_postargs) try: self.spawn(compiler_so + cc_args + [src, '-o', obj] + extra_postargs) except DistutilsExecError as msg: > raise CompileError(msg) E distutils.errors.CompileError: command 'gcc' failed with exit status 1 ../pypy/lib-python/3/distutils/unixccompiler.py:135: CompileError ------------------------------------------------------------------- Captured stderr call ------------------------------------------------------------------- /home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.c: In function ‘ctx_Dump’: /home/matti/oss/hpy/hpy/devel/src/runtime/ctx_object.c:15:5: error: implicit declaration of function ‘_PyObject_Dump’; did you mean ‘PyObject_Del’? [-Werror=implicit-function-declaration] 15 | _PyObject_Dump(_h2py(h)); | ^~~~~~~~~~~~~~ | PyObject_Del compilation terminated due to -Wfatal-errors. cc1: all warnings being treated as errors ================================================================= short test summary info ================================================================== FAILED test/test_00_basic.py::TestBasic::test_empty_module[cpython] - distutils.errors.CompileError: command 'gcc' failed with exit status 1 ```

[hodgestar]

@antocuni Do you know how to fix this on PyPy or what went wrong here?

[rlamy]

@mattip HPy-for-PyPy is implemented in the pypy repo. The tests in this repo aren't meant to run on pypy. The tests you want to run are in <pypy repo>/extra_tests/hpy_tests.

[mattip]

1. So running tests here with PyPy/GraalPython should fail early
2. Will the complete test suite be in extra_tests/hpy_tests or only ones ported from here by hand?

[antocuni]

The use of CPython internals inside HPy code is problematic. Building in the universal mode should probably define the limited API.

No, I think there is a bit of confusion here. When we talk about HPy on CPython, there are two modules involved:

• hpy/universal.cpython-38-x86_64-linux-gnu.so (or similar): this is CPython-specific and will have to be recompiled for every version of CPython. We could in theory try to use the stable ABI for it, but is it worth? The module itself is very small and quick to compile, and we need to test hpy for every cpython version that we want to support anyway.

Otherwise the universal binaries will not really be universal, they may fail on some versions of CPython.

I don't understand how. If your extension calls HPy_Dump and you compile in universal mode, it will contain an opaque call to ctx->ctx_Dump() and it will never see _PyObject_Dump() (which is inside universal.cpython-38-x86_64-linux-gnu.so and it's cpython-specific.

---

1. So running tests here with PyPy/GraalPython should fail early

yes! I think we should:

1. add a compile-time check to e.g. hpy/devel/include/hpy/cpython/misc.h which fails if we are compiling it with pypy, similar to this: https://github.com/hpyproject/hpy/blob/38893482e7443fb9924b49d381a1f052b2525e96/hpy/universal/src/hpymodule.c#L16-L21

2. add an early check in our test suite which refuses to run tests on top of pypy (and probably graalpython)

2. Will the complete test suite be in extra_tests/hpy_tests or only ones ported from here by hand?

There is a script which automatically copies all the vendored files from the hpy repo to the pypy repo, including the whole test suite: https://foss.heptapod.net/pypy/pypy/-/blob/branch/hpy-0.0.4/pypy/module/_hpy_universal/update_vendored.sh#L129