From 0697f8d8cffa45346c4e85f3753a5cc5e48f5c06 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. Skip the CppInterOp ObjToString call. Its toString is an assert(0) stub, so the call aborts with assertions on and returns "" without them. op_str now falls back to the generic repr on every build. Test the repr fallback in process. The earlier subprocess test is no longer necessary, because the path does not crash. Retire the three xfail markers that named the toString crash. Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop) Co-developed-with-the-help-of: Claude Code (Opus 5, human in the loop) Co-developed-with-the-help-of: OpenAI Codex (GPT-6, human in the loop) --- CMakeLists.txt | 1 + src/cpyrt/CPPInstance.cxx | 20 +++------------ src/cpyrt/Compatibility.h | 48 +++++++++++++++++++++++++++++++++++ test/test_crossinheritance.py | 5 ---- test/test_doc_features.py | 6 ----- test/test_regression.py | 18 ++++++++++--- 6 files changed, 66 insertions(+), 32 deletions(-) create mode 100644 src/cpyrt/Compatibility.h diff --git a/CMakeLists.txt b/CMakeLists.txt index c3f8ed8..c32cae7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -76,6 +76,7 @@ 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}" + $<$:CPPJIT_USE_CLING> ) target_include_directories(cppjit PRIVATE diff --git a/src/cpyrt/CPPInstance.cxx b/src/cpyrt/CPPInstance.cxx index 425c654..0456669 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" @@ -841,7 +842,7 @@ static PyObject* op_str(CPPInstance* self) { // otherwise not found // TODO: ToString() still leaks ... const std::string& pretty = - interop::ToString(self->ObjectIsA(), self->GetObject()); + compat::ObjToString(self->ObjectIsA(), self->GetObject()); if (!pretty.empty()) return cpyrt_PyText_FromString(pretty.c_str()); continue; @@ -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..e9771d7 --- /dev/null +++ b/src/cpyrt/Compatibility.h @@ -0,0 +1,48 @@ +#ifndef CPYRT_COMPATIBILITY_H +#define CPYRT_COMPATIBILITY_H + +#include "Python.h" + +#include "cppjit_interop.h" + +#include + +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 +} + +// Interpreter::toString is an assert(0) stub (CppInterOp#1100); skip it. +inline std::string ObjToString(interop::TCppScope_t /*klass*/, + interop::TCppObject_t /*obj*/) { + return ""; +} + +} // namespace cppjit::cpyrt::compat + +#endif // CPYRT_COMPATIBILITY_H diff --git a/test/test_crossinheritance.py b/test/test_crossinheritance.py index 0e405de..2e537df 100644 --- a/test/test_crossinheritance.py +++ b/test/test_crossinheritance.py @@ -3,7 +3,6 @@ import py from pytest import mark, raises, skip from support import ( - IS_CLANG_DEBUG, IS_CLING, IS_LINUX_ARM, IS_MAC, @@ -27,10 +26,6 @@ def setup_class(cls): cls.example01 = cppjit.load_reflection_info(cls.test_dct) - @mark.xfail( - run=not (IS_CLANG_DEBUG or IS_CLING), - reason="Crashes with ClangRepl with 'toString not implemented' and on Cling", - ) def test01_override_function(self): """Test ability to override a simple function""" diff --git a/test/test_doc_features.py b/test/test_doc_features.py index fd15cdc..52bb067 100644 --- a/test/test_doc_features.py +++ b/test/test_doc_features.py @@ -3,7 +3,6 @@ import py from pytest import mark, raises, skip from support import ( - IS_CLANG_REPL, IS_CLING, IS_LINUX_ARM, IS_MAC, @@ -1040,11 +1039,6 @@ def test08_voidptr_array(self): assert n.p[2] == 0x3 assert len(n.p) == 3 - @mark.xfail( - condition=IS_CLANG_REPL and IS_MAC, - run=False, - reason="Crashes with ClangRepl with 'toString not implemented'", - ) def test09_custom_str(self): """Example of customized str""" diff --git a/test/test_regression.py b/test/test_regression.py index 1add840..c82d808 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -1040,10 +1040,6 @@ class ReferenceWavefunction {}; cppjit.gbl.std.get[0](cppjit.gbl.property_types.run_as[pt_type]()) == 20.0 ) - @mark.xfail( - run=False, - reason="Crashes on ClangRepl with 'toString not implemented', and on Cling", - ) def test34_print_empty_collection(self): """Print empty collection through Cling""" @@ -1648,3 +1644,17 @@ 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""" + + import cppjit + + cppjit.cppdef("namespace StrFallback { struct Bare { int x; }; }") + + bare = cppjit.gbl.StrFallback.Bare() + + # no ostream inserter and no usable pretty printer, so op_str must + # degrade to the generic repr + assert str(bare) == repr(bare) + assert "Bare object at" in str(bare)