From 85b8e7c0c84f53c4a2467c51d8d9a3bfd7f4f90f Mon Sep 17 00:00:00 2001 From: leftibot Date: Sat, 2 May 2026 11:38:01 -0600 Subject: [PATCH] Fix #633: [Bug] Stack-overflow due to infinite recursion in user-defined operator (string interpolation) (#700) * Fix #633: Bound ChaiScript call stack to prevent native stack overflow Recursive user-defined operators (e.g. a `string::/=` whose body calls itself through string interpolation) drove the AST evaluator into unbounded native recursion and crashed the host process with SIGSEGV. The dispatcher now refuses to enter a new function frame once `Stack_Holder::call_depth` reaches `chaiscript::max_call_depth` (default 256, overridable via the `CHAISCRIPT_MAX_CALL_DEPTH` macro) and throws the new `chaiscript::exception::stack_overflow_error` instead, letting both ChaiScript-level `try`/`catch` and C++ hosts recover from runaway recursion. Co-Authored-By: Claude Opus 4.7 (1M context) * Address review: tighten max_call_depth on MSVC Debug Windows MSVC Debug builds crashed unit.recursion_depth_protection.chai with SEGFAULT before the depth check could fire. Windows defaults to a 1 MiB thread stack and Debug builds emit much larger per-frame native stack usage (no inlining, /RTC, buffer security checks), so 256 nested ChaiScript calls overflow the native stack long before the dispatcher reaches max_call_depth. Linux/macOS and MSVC Release pass at 256. Pick a tighter default (32) only for the MSVC + _DEBUG configuration that needs it, leaving every other build at the original 256, and shrink the count_down recursion in the regression test so the bounded path stays well below every platform's default. Requested by @lefticus in PR #700 review. Co-Authored-By: Claude Opus 4.7 (1M context) * Address review: use CHAISCRIPT_DEBUG instead of _DEBUG Switch CHAISCRIPT_DEBUG from a true/false definition to 1/0 so it can be used in preprocessor #if expressions, then reuse it for the MSVC Debug guard around CHAISCRIPT_MAX_CALL_DEPTH instead of testing the compiler-private _DEBUG macro directly. The C++ debug_build constant keeps its bool value through implicit conversion. Requested by @lefticus in PR #700 review. Co-Authored-By: Claude Opus 4.7 (1M context) * Address review: restore CHAISCRIPT_DEBUG to true/false for stronger typing C++ preserves the true/false keywords in #if directives ([cpp.cond]), so the MSVC Debug guard around CHAISCRIPT_MAX_CALL_DEPTH still works without weakening the macro to integer 1/0. Requested by @lefticus in PR #700 review. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: leftibot Co-authored-by: Claude Opus 4.7 (1M context) --- include/chaiscript/chaiscript_defines.hpp | 21 ++++++++++ .../chaiscript/dispatchkit/dispatchkit.hpp | 17 ++++++++ unittests/recursion_depth_protection.chai | 41 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 unittests/recursion_depth_protection.chai diff --git a/include/chaiscript/chaiscript_defines.hpp b/include/chaiscript/chaiscript_defines.hpp index 530fb419..f8677ec8 100644 --- a/include/chaiscript/chaiscript_defines.hpp +++ b/include/chaiscript/chaiscript_defines.hpp @@ -75,6 +75,24 @@ static_assert(_MSC_FULL_VER >= 190024210, "Visual C++ 2015 Update 3 or later req #define CHAISCRIPT_DEBUG false #endif +// Upper bound on the depth of nested ChaiScript function calls. Hitting it +// causes the dispatcher to throw chaiscript::exception::stack_overflow_error +// instead of letting the native call stack overflow. Defining the macro on +// the command line overrides the default. +// +// MSVC Debug builds emit very large per-frame native stack usage (no inlining, +// /RTC, buffer security checks) and Windows defaults to a 1 MiB thread stack, +// so the same ChaiScript depth that fits comfortably on Linux/macOS or in an +// MSVC Release build overflows the native stack before the depth check fires. +// We pick a tighter default in that configuration to keep the throw reachable. +#ifndef CHAISCRIPT_MAX_CALL_DEPTH +#if defined(CHAISCRIPT_MSVC) && CHAISCRIPT_DEBUG +#define CHAISCRIPT_MAX_CALL_DEPTH 32 +#else +#define CHAISCRIPT_MAX_CALL_DEPTH 256 +#endif +#endif + #include #include #include @@ -88,6 +106,9 @@ namespace chaiscript { constexpr static const char *compiler_name = CHAISCRIPT_COMPILER_NAME; constexpr static const bool debug_build = CHAISCRIPT_DEBUG; + constexpr static const int max_call_depth = CHAISCRIPT_MAX_CALL_DEPTH; + static_assert(max_call_depth > 0, "CHAISCRIPT_MAX_CALL_DEPTH must be a positive integer"); + template inline std::shared_ptr make_shared(Arg &&...arg) { #ifdef CHAISCRIPT_USE_STD_MAKE_SHARED diff --git a/include/chaiscript/dispatchkit/dispatchkit.hpp b/include/chaiscript/dispatchkit/dispatchkit.hpp index 2d1eff7e..f38147c0 100644 --- a/include/chaiscript/dispatchkit/dispatchkit.hpp +++ b/include/chaiscript/dispatchkit/dispatchkit.hpp @@ -121,6 +121,19 @@ namespace chaiscript { global_non_const(const global_non_const &) = default; ~global_non_const() noexcept override = default; }; + + /// Exception thrown when the ChaiScript call stack exceeds + /// chaiscript::max_call_depth, signalling runaway recursion before the + /// native stack can overflow. + class stack_overflow_error : public std::runtime_error { + public: + stack_overflow_error() noexcept + : std::runtime_error("Maximum call stack depth exceeded") { + } + + stack_overflow_error(const stack_overflow_error &) = default; + ~stack_overflow_error() noexcept override = default; + }; } // namespace exception /// \brief Holds a collection of ChaiScript settings which can be applied to the ChaiScript runtime. @@ -1010,6 +1023,10 @@ namespace chaiscript { void save_function_params(const Function_Params &t_params) { save_function_params(*m_stack_holder, t_params); } void new_function_call(Stack_Holder &t_s, Type_Conversions::Conversion_Saves &t_saves) { + if (t_s.call_depth >= max_call_depth) { + throw chaiscript::exception::stack_overflow_error{}; + } + if (t_s.call_depth == 0) { m_conversions.enable_conversion_saves(t_saves, true); } diff --git a/unittests/recursion_depth_protection.chai b/unittests/recursion_depth_protection.chai new file mode 100644 index 00000000..716edb9a --- /dev/null +++ b/unittests/recursion_depth_protection.chai @@ -0,0 +1,41 @@ +// Regression test for issue #633: Stack-overflow due to infinite recursion +// in user-defined operator (string interpolation). +// +// Before the fix the recursive `/=` invocation triggered unbounded native +// recursion in the evaluator and crashed the host process with a SIGSEGV. +// The engine now bounds call_depth and throws a catchable exception +// instead of letting the native stack overflow. + +def string::`/=`(double d) { + this = "${this/= 2}/=${d}"; + return this; +} + +var s = "o World" +var caught = false +var message = "" + +try { + s /= 2 + // unreachable: the recursive operator must abort with an exception + assert_true(false) +} catch (e) { + caught = true + message = e.what() +} + +assert_true(caught) + +// The reported error must mention the call-stack overflow so users can +// distinguish it from an arbitrary script-level error. +assert_true(find(message, "call stack") != -1) + +// A bounded recursion that stays well below the limit must keep working. +// Use a small depth so the test passes on every platform default - notably +// the MSVC Debug build where the native stack budget forces a tighter cap. +def count_down(n) { + if (n <= 0) { return 0 } + return count_down(n - 1) + 1 +} + +assert_equal(10, count_down(10))