Make the behavior of USE_STACKCHECK deterministic #76038

Activity

1. pdox commented on Oct 24, 2017

   

   pdoxmannequin

   on Oct 24, 2017

   MannequinAuthor

   More actions

   USE_STACKCHECK is a Windows-only feature which provides additional safety against C stack overflow by periodically calling PyOS_CheckStack to determine whether the current thread is too close to the end of the stack.

   The way USE_STACKCHECK ensures that PyOS_CheckStack is called frequently is surprising. It does this by artificially decrementing _Py_CheckRecursionLimit with every call to Py_EnterRecursiveCall:

       #ifdef USE_STACKCHECK
       /* With USE_STACKCHECK, we artificially decrement the recursion limit in order
          to trigger regular stack checks in _Py_CheckRecursiveCall(), except if
          the "overflowed" flag is set, in which case we need the true value
          of _Py_CheckRecursionLimit for _Py_MakeEndRecCheck() to function properly.
       */
       #  define _Py_MakeRecCheck(x)  \
           (++(x) > (_Py_CheckRecursionLimit += PyThreadState_GET()->overflowed - 1))
       #else
       #  define _Py_MakeRecCheck(x)  (++(x) > _Py_CheckRecursionLimit)
       #endif

   _Py_CheckRecursionLimit defaults to 1000, and is constant when USE_STACKCHECK is off. (except when changed by Py_SetRecursionLimit)

   With a single thread, each call to Py_EnterRecursiveCall causes _Py_CheckRecursionLimit to decrement by 1 until it equals the current recursion depth, at which point _Py_CheckRecursiveCall is triggered, resetting _Py_CheckRecursionLimit back to the default. This could be anywhere from 500 to 1000 calls depending on the recursion depth. With multiple threads, the behavior may be more chaotic, as each thread will be decrementing _Py_CheckRecursionLimit independently.

   I propose that instead, the call to PyOS_CheckStack is triggered in a predictable fashion, using a separate counter for each thread, so that it occurs every N'th call to Py_EnterRecursiveCall. (N = 64 seems reasonable, as PyOS_CheckStack guarantees 2048 * sizeof(void*) bytes remaining on the C stack).
2. pdox

   added

   3.7 (EOL)end of lifeend of life

   OS-windows

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Oct 24, 2017
3. benjaminp commented on Oct 26, 2017

   

   benjaminp

   on Oct 26, 2017

   Contributor

   More actions

   New changeset 1896793 by Benjamin Peterson (pdox) in branch 'master':
   bpo-31857: Make the behavior of USE_STACKCHECK deterministic (bpo-4098)
   1896793
4. benjaminp

   closed this as completedon Oct 26, 2017
5. vstinner commented on Nov 4, 2019

   

   vstinner

   on Nov 4, 2019

   Member

   More actions

   This issue added this FIXME:

   /* Due to the macros in which it's used, _Py_CheckRecursionLimit is in
   the stable ABI. It should be removed therefrom when possible.
   */

   FYI I proposed PR 17046 to fix it ;-)
6. ezio-melotti

   transferred this issue fromon Apr 10, 2022