From 798756448180195a6ce020565a5c1d160e491e98 Mon Sep 17 00:00:00 2001 From: "Erik M. Bray" Date: Thu, 6 Dec 2018 16:11:35 +0000 Subject: [PATCH 3/3] Prototype for GAP_Enter/Leave macros to bracket use of libgap and stack local GAP objects in code which embeds libgap There are two parts to this: First, the outer-most GAP_Enter() must set the StackBottom variable for GASMAN, without which objects tracked by GASMAN that are allocated on the stack are properly tracked (see #3089). Second, the outer-most GAP_Enter() call should set a jump point for longjmps to STATE(ReadJmpError). Code within the GAP kernel may reset this, but we should set it here in case any unexpected errors occur within the GAP kernel that are not already handled appropriately by a TRY_IF_NO_ERROR. For the first issue, we add GAP_EnterStack() and GAP_LeaveStack() macros which implement *just* the StackBottom handling without any other error handling. We also add a function to gasman.c called _MarkStackBottomBags which just updates the StackBottom variable. Then the macro called MarkStackBottomBags (same name without underscore) can be used within a function to set StackBottom to somewhere at or near the beginning of that function's stack frame. This uses GCC's __builtin_frame_address, but supported is probably needed for other platforms that don't have this. The state variable STATE(EnterStackCount) is used to track recursive calls into GAP_EnterStack(). We only want to set StackBottom on the outer-most call. The count is decremented on GAP_LeaveStack(). Some functions are provided for manipulating the counter from the API without directly exposing the GAP state, but I'm not sure if this is necessary or desirable, especially since it means EnterStackCount isn't updated atomically. My hope was to avoid exposing too many GAP internals, but it may be necessary in order to implement these as macros. For setting the STATE(ReadJmpError) jump buffer we provide a macro called GAP_Error_Setjmp() which is fairly straightforward, except that it needs to be written in such a way that it can be used in concert correctly with GAP_EnterStack(). In particular, if returning to the site of a GAP_Error_Setjmp() call we do not want to accidentally re-increment the EnterStackCount. Finally, the higher-level GAP_Enter() and GAP_Leave() macros are provided: The latter is just an alias for GAP_LeaveStack(), but the former carefully combines *both* GAP_Error_Setjmp() and GAP_EnterStack(). Although called like a function, the GAP_Enter() macro expands to a compound statement (necessary for both GAP_EnterStack() and GAP_Error_Setjmp() to work properly). The order of expansion is also deliberate so that this can be used like: jmp_retval = GAP_Enter(); so that the return value of the setjmp call can be stored in a variable while using GAP_Enter(), and can be checked to see whether an error occurred. However, this requires some care to ensure that the following GAP_EnterStack() doesn't increment the EnterStackCount following a return to this point via a longjmp. Conflicts: src/libgap-api.h --- src/gapstate.h | 3 ++ src/gasman.c | 6 ++++ src/gasman.h | 15 ++++++++++ src/libgap-api.c | 27 +++++++++++++++++ src/libgap-api.h | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 137 insertions(+), 2 deletions(-) diff --git a/src/gapstate.h b/src/gapstate.h index 72ec4c3..7a2a663 100644 --- a/src/gapstate.h +++ b/src/gapstate.h @@ -97,6 +97,9 @@ typedef struct GAPState { UInt1 StateSlots[STATE_SLOTS_SIZE]; + /* For libgap-api.c */ + Int EnterStackCount; + /* Allocation */ #if !defined(USE_GASMAN) #define MAX_GC_PREFIX_DESC 4 diff --git a/src/gasman.c b/src/gasman.c index 13c0b1e..4d2ab3d 100644 --- a/src/gasman.c +++ b/src/gasman.c @@ -1193,6 +1193,12 @@ void SetExtraMarkFuncBags(TNumExtraMarkFuncBags func) } + +void _MarkStackBottomBags(void* StackBottom) { + StackBottomBags = StackBottom; +} + + void InitBags ( UInt initial_size, Bag * stack_bottom, diff --git a/src/gasman.h b/src/gasman.h index 236eb8b..55e057a 100644 --- a/src/gasman.h +++ b/src/gasman.h @@ -982,6 +982,21 @@ extern void InitCollectFuncBags ( typedef void (*TNumExtraMarkFuncBags)(void); extern void SetExtraMarkFuncBags(TNumExtraMarkFuncBags func); + +#ifdef __GNUC__ +#define MarkStackBottomBags() \ + _MarkStackBottomBags(__builtin_frame_address(0)); +/* +#else + * TODO: Detect the best stack frame detection technique at configure time + * +#define MarkStackBottomBags() \ + register void* rbp asm("rbp"); \ + _MarkStackBottomBags(rbp); +*/ +#endif +extern void _MarkStackBottomBags(void* StackBottom); + /**************************************************************************** ** *F InitBags(...) . . . . . . . . . . . . . . . . . . . . . initialize Gasman diff --git a/src/libgap-api.c b/src/libgap-api.c index 82cc441..e75b0e2 100644 --- a/src/libgap-api.c +++ b/src/libgap-api.c @@ -10,6 +10,8 @@ #include "lists.h" #include "streams.h" #include "stringobj.h" +#include "system.h" + // // Setup and initialisation @@ -60,3 +62,28 @@ Obj GAP_EvalString(const char * cmd) res = READ_ALL_COMMANDS(instream, False, True, viewObjFunc); return res; } + +inline syJmp_buf * _GAP_GetReadJmpError(void) +{ + return &(STATE(ReadJmpError)); +} + +inline Int _GAP_GetEnterStackCount(void) +{ + return STATE(EnterStackCount); +} + +inline void _GAP_IncEnterStackCount(void) +{ + STATE(EnterStackCount)++; +} + +inline void _GAP_DecEnterStackCount(void) +{ + STATE(EnterStackCount)--; +} + +inline void _GAP_SetEnterStackCount(Int count) +{ + STATE(EnterStackCount) = count; +} diff --git a/src/libgap-api.h b/src/libgap-api.h index e45d6fc..55fcd05 100644 --- a/src/libgap-api.h +++ b/src/libgap-api.h @@ -5,9 +5,93 @@ #include "gap.h" -typedef void (*CallbackFunc)(void); +#ifdef __GNUC__ +#define unlikely(x) __builtin_expect(!!(x), 0) +#else +#define unlikely(x) (x) +#endif + + +#ifndef GAP_ENTER_DEBUG +#define GAP_ENTER_DEBUG 0 +#endif + + +extern syJmp_buf * _GAP_GetReadJmpError(void); +extern Int _GAP_GetEnterStackCount(void); +extern void _GAP_IncEnterStackCount(void); +extern void _GAP_DecEnterStackCount(void); +extern void _GAP_SetEnterStackCount(Int count); + + +#if GAP_ENTER_DEBUG +#define GAP_ENTER_DEBUG_MESSAGE(message, file, line) \ + fprintf(stderr, "%s: %d; %s:%d\n", message, _GAP_EnterStackCount, file, line); +#else +#define GAP_ENTER_DEBUG_MESSAGE(message, file, line) +#endif + + +#define GAP_EnterStack() \ + GAP_ENTER_DEBUG_MESSAGE("EnterStack", __FILE__, __LINE__); \ + Int _gap_tmp_enter_stack_count = _GAP_GetEnterStackCount(); \ + if (_gap_tmp_enter_stack_count < 0) { \ + _GAP_SetEnterStackCount(-_gap_tmp_enter_stack_count); \ + } else { \ + if (_gap_tmp_enter_stack_count == 0) { \ + MarkStackBottomBags(); \ + } \ + _GAP_IncEnterStackCount(); \ + } + -// Initialisation and finalization +#define GAP_LeaveStack() \ + _GAP_DecEnterStackCount(); \ + GAP_ENTER_DEBUG_MESSAGE("LeaveStack", __FILE__, __LINE__); + + +static inline int _GAP_Error_Prejmp(const char* file, int line) { +#if GAP_ENTER_DEBUG + GAP_ENTER_DEBUG_MESSAGE("Error_Prejmp", file, line); +#endif + if (_GAP_GetEnterStackCount() > 0) { + return 1; + } + return 0; +} + + +static inline int _GAP_Error_Postjmp(int JumpRet) +{ + if (unlikely(JumpRet != 0)) { + /* This only should have been called from the outer-most + * GAP_EnterStack() call so make sure it resets the EnterStackCount; We + * set EnterStackCount to its negative which indicates to + * GAP_EnterStack that we just returned from a long jump and should + * reset EnterStackCount to its value at the return point rather than + * increment it again */ + Int tmp_count = _GAP_GetEnterStackCount(); + if (tmp_count > 0) { + _GAP_SetEnterStackCount(-tmp_count); + } + return 0; + } + + return 1; +} + +#define GAP_Error_Setjmp() (_GAP_Error_Prejmp(__FILE__, __LINE__) || \ + _GAP_Error_Postjmp(sySetjmp(*_GAP_GetReadJmpError()))) + + +#define GAP_Enter() GAP_Error_Setjmp(); GAP_EnterStack() +#define GAP_Leave() GAP_LeaveStack() + + +//// +//// Setup and initialisation +//// +typedef void (*CallbackFunc)(void); void GAP_Initialize(int argc, char ** argv, -- 1.9.1