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]) + )