From e04312953b437b53ca90b7c1884a348fffff5e94 Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Tue, 25 Aug 2026 15:14:24 -0500 Subject: [PATCH] cpyrt: null-guard the cling.printValue lookup in op_str Commit 5a33027 dropped the faked cling::runtime::gCling and with it the last cling namespace in the interpreter. The pretty-print fallback in op_str then dereferenced the failed cppjit.gbl.cling lookup. str() of any instance without an ostream inserter crashed with SIGSEGV. Guard both lookups, clear the AttributeError, and fall back to the generic repr. clang-repl has no cling namespace to look up, so the whole pretty-print path now compiles out behind CPPJIT_USE_CLING. CMake gains the matching compile definition: the option existed but never reached the sources. The null-guards stay on the cling side. This drops the old escape hatch where a user-declared namespace cling { printValue } was honored on clang-repl, which matches the native value-printing direction in compiler-research/CppInterOp#1100. The regression test runs the repro in a subprocess, so a return of the crash cannot kill the test runner. The subprocess gets this process's sys.path through PYTHONPATH, because a build system can put cppjit on the path in-process instead. A clean exit passes, whichever repr form the backend prints. Output that names the upstream toString stub xfails. Anything else fails and reports the child's output. Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop) --- CMakeLists.txt | 2 ++ src/cpyrt/CPPInstance.cxx | 18 +++++++++---- test/test_regression.py | 57 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 465d158..8fe1c48 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -129,6 +129,8 @@ target_compile_definitions(cppjit PRIVATE CPPINTEROP_INCLUDE_DIR="interop/include" CPPJIT_CLANG_MAJOR="${LLVM_VERSION_MAJOR}" CPPJIT_CLANG_INCLUDE_DIR="interop/lib/clang/${LLVM_VERSION_MAJOR}" + # cling-only code paths need the flavor at compile time, not just in cmake + $<$:CPPJIT_USE_CLING> ) target_include_directories(cppjit PRIVATE diff --git a/src/cpyrt/CPPInstance.cxx b/src/cpyrt/CPPInstance.cxx index 59cde8a..edd2344 100644 --- a/src/cpyrt/CPPInstance.cxx +++ b/src/cpyrt/CPPInstance.cxx @@ -869,23 +869,30 @@ static PyObject* op_str(CPPInstance* self) { } // 2. Cling's pretty printing (not done through backend for performance - // reasons) + // reasons). Cling only: clang-repl has no cling namespace to look up, so the + // whole path compiles out and str() falls through to the generic repr. +#ifdef CPPJIT_USE_CLING if (!ScopeFlagCheck(self, CPPScope::kNoPrettyPrint)) { static PyObject* printValue = nullptr; if (!printValue) { PyObject* gbl = PyDict_GetItemString(PySys_GetObject((char*)"modules"), "cppjit.gbl"); - PyObject* cl = PyObject_GetAttrString(gbl, (char*)"cling"); - printValue = PyObject_GetAttrString(cl, (char*)"printValue"); - Py_DECREF(cl); + // no cling namespace exists unless user code declares one + PyObject* cl = + gbl ? PyObject_GetAttrString(gbl, (char*)"cling") : nullptr; + printValue = + cl ? PyObject_GetAttrString(cl, (char*)"printValue") : nullptr; + Py_XDECREF(cl); // gbl is borrowed if (printValue) { Py_DECREF(printValue); // make borrowed if (!PyCallable_Check(printValue)) printValue = nullptr; // unusable ... } - if (!printValue) // unlikely + if (!printValue) { + PyErr_Clear(); ScopeFlagSet(self, CPPScope::kNoPrettyPrint); + } } if (printValue) { @@ -929,6 +936,7 @@ static PyObject* op_str(CPPInstance* self) { // if not available/specialized, don't try again ScopeFlagSet(self, CPPScope::kNoPrettyPrint); } +#endif // CPPJIT_USE_CLING // 3. Generic printing as done in op_repr return op_repr(self); diff --git a/test/test_regression.py b/test/test_regression.py index 638fc50..df90a6d 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -1,7 +1,7 @@ import os import sys -from pytest import mark, raises, skip +from pytest import mark, raises, skip, xfail from support import ( IS_CLANG_REPL, IS_CLING, @@ -1637,3 +1637,58 @@ def test51_nontype_enum_template_arg(self): # ...nor leave the interpreter unable to compile a later call wrapper assert ns.probe(41) == 42 + + def test52_str_fallback_without_ostream_insertion(self): + """str() of an instance with no operator<< used to crash. + + With no ``cling`` namespace in the interpreter, the pretty-print + fallback dereferenced the failed ``cppjit.gbl.cling`` lookup and the + process died. A regression is therefore fatal, not an assertion + failure, so run the repro in a subprocess: the runner survives and the + output identifies which failure happened. + """ + + import os + import subprocess + import sys + + repro = """\ +import cppjit + +cppjit.cppdef("namespace StrFallback { struct Bare { int x; }; }") +print(repr(str(cppjit.gbl.StrFallback.Bare()))) +""" + + # A build system can put cppjit on sys.path without PYTHONPATH (bazel + # gives the runner a bootstrap instead), so hand the child this + # process's own path. + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join(p for p in sys.path if p) + + popen = subprocess.Popen( + [sys.executable, "-c", repro], + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + ) + stdout, _ = popen.communicate() + output = stdout.decode("utf-8", "replace") + + # the guard holds: cling prints the @0xADDR form through printValue and + # ClangRepl falls back to the generic repr, and neither crashes + if popen.returncode == 0: + return + + # Interpreter::toString is an assert(0) stub upstream. str() tries the + # ostream path first, which reaches it whenever assertions are on. + if "toString is not implemented" in output: + xfail( + "toString stub aborts, see compiler-research/CppInterOp#1100: " + "%s" % (output[:300],) + ) + + # a crash banner and its top frames come first, so keep the head + raise AssertionError( + "str() without an ostream inserter did not fall back cleanly: " + "returncode=%s output=%r" % (popen.returncode, output[:2000]) + )