From 53ae578ec1b06d0a2da8e286ad156cccde47ee1f Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Tue, 25 Aug 2026 15:14:24 -0500 Subject: [PATCH] cpyrt: guard the Cling printValue lookup Prevent a null dereference when str() cannot find cling.printValue. Keep the interpreter check and cached lookup in Compatibility.h. The helper returns nullptr on clang-repl and preserves the Cling fallback. Pass CPPJIT_USE_CLING from CMake to the sources. Run the regression in a subprocess and pass sys.path through PYTHONPATH. Retain the expected failure for the CppInterOp toString stub. Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop) Co-developed-with-the-help-of: OpenAI Codex (GPT-6, human in the loop) --- CMakeLists.txt | 2 ++ src/cpyrt/CPPInstance.cxx | 18 ++----------- src/cpyrt/Compatibility.h | 38 ++++++++++++++++++++++++++ test/test_regression.py | 57 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 src/cpyrt/Compatibility.h diff --git a/CMakeLists.txt b/CMakeLists.txt index c3f8ed8..ea1d5ee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -76,6 +76,8 @@ target_compile_definitions(cppjit PRIVATE CPPINTEROP_INCLUDE_DIR="${CPPJIT_INTEROP_RUNTIME_INCLUDES}" CPPJIT_CLANG_MAJOR="${CPPJIT_INTEROP_CLANG_MAJOR}" CPPJIT_CLANG_INCLUDE_DIR="${CPPJIT_INTEROP_CLANG_DIR}" + # 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 425c654..9bda2f6 100644 --- a/src/cpyrt/CPPInstance.cxx +++ b/src/cpyrt/CPPInstance.cxx @@ -5,6 +5,7 @@ using namespace cppjit; #include "CPPInstance.h" #include "CPPOverload.h" #include "CPPScope.h" +#include "Compatibility.h" #include "MemoryRegulator.h" #include "ProxyWrappers.h" #include "PyStrings.h" @@ -870,22 +871,7 @@ static PyObject* op_str(CPPInstance* self) { // 2. Cling's pretty printing (not done through backend for performance // reasons) 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); - // gbl is borrowed - if (printValue) { - Py_DECREF(printValue); // make borrowed - if (!PyCallable_Check(printValue)) - printValue = nullptr; // unusable ... - } - if (!printValue) // unlikely - ScopeFlagSet(self, CPPScope::kNoPrettyPrint); - } + PyObject* printValue = compat::GetClingPrintValue(); if (printValue) { // as printValue only works well for templates taking pointer arguments, diff --git a/src/cpyrt/Compatibility.h b/src/cpyrt/Compatibility.h new file mode 100644 index 0000000..61d86d3 --- /dev/null +++ b/src/cpyrt/Compatibility.h @@ -0,0 +1,38 @@ +#ifndef CPYRT_COMPATIBILITY_H +#define CPYRT_COMPATIBILITY_H + +#include "Python.h" + +namespace cppjit::cpyrt::compat { + +inline PyObject* GetClingPrintValue() { +#ifdef CPPJIT_USE_CLING + static PyObject* printValue = nullptr; + if (printValue) + return printValue; + + PyObject* gbl = + PyDict_GetItemString(PySys_GetObject((char*)"modules"), "cppjit.gbl"); + PyObject* cling = gbl ? PyObject_GetAttrString(gbl, (char*)"cling") : nullptr; + printValue = + cling ? PyObject_GetAttrString(cling, (char*)"printValue") : nullptr; + Py_XDECREF(cling); + + if (printValue) { + Py_DECREF(printValue); // make borrowed + if (!PyCallable_Check(printValue)) + printValue = nullptr; + } + + if (!printValue) + PyErr_Clear(); + + return printValue; +#else + return nullptr; +#endif +} + +} // namespace cppjit::cpyrt::compat + +#endif // CPYRT_COMPATIBILITY_H diff --git a/test/test_regression.py b/test/test_regression.py index 1add840..4f65b47 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 ( CAN_JIT_STD_FILESYSTEM, IS_CLANG_REPL, @@ -1648,3 +1648,58 @@ def test52_no_cpp_name_for_template_arg(self): with raises(TypeError): cppjit.gbl.std.vector[object()] + + def test53_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]) + )