From c4b377511c05c172e3b27e5064429364c555867e Mon Sep 17 00:00:00 2001 From: Graham Chapman Date: Thu, 27 Oct 2022 11:42:46 -0400 Subject: [PATCH] Stack walk loop breaker for ASGCT Prevent potential endless loop in the stack walker when examining the stack of a thread which has been interrupted when not at a safe point. Signed-off-by: Graham Chapman --- runtime/oti/j9consts.h | 14 +++---- runtime/oti/j9nonbuilder.h | 1 + runtime/vm/swalk.c | 83 ++++++++++++++++++++++++++------------ 3 files changed, 64 insertions(+), 34 deletions(-) diff --git a/runtime/oti/j9consts.h b/runtime/oti/j9consts.h index 78ec6e0515c..0a20b449261 100644 --- a/runtime/oti/j9consts.h +++ b/runtime/oti/j9consts.h @@ -375,16 +375,14 @@ extern "C" { #define J9_STARTPC_DLT_READY 0x8 #define J9_STARTPC_STATUS 0xF -/* Frame iterator return values */ -#define J9_STACKWALK_STOP_ITERATING 0x0 -#define J9_STACKWALK_KEEP_ITERATING 0x1 - /* Stackwalk return values */ #define J9_STACKWALK_RC_NONE 0x0 -#define J9_STACKWALK_RC_NO_MEMORY 0x1 -#define J9_STACKWALK_RC_FRAME_NOT_FOUND 0x2 -#define J9_STACKWALK_RC_BAD_STATE_BUFFER 0x3 -#define J9_STACKWALK_RC_STACK_CORRUPT 0x4 +#define J9_STACKWALK_STOP_ITERATING 0x0 /* Must be the same as no error */ +#define J9_STACKWALK_KEEP_ITERATING 0x1 +#define J9_STACKWALK_RC_NO_MEMORY 0x2 +#define J9_STACKWALK_RC_FRAME_NOT_FOUND 0x3 +#define J9_STACKWALK_RC_BAD_STATE_BUFFER 0x4 +#define J9_STACKWALK_RC_STACK_CORRUPT 0x5 /* Tag for JIT artifact search cache */ #define J9_STACKWALK_NO_JIT_CACHE 0x1 diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h index d0dc9689749..afb52b8d11f 100644 --- a/runtime/oti/j9nonbuilder.h +++ b/runtime/oti/j9nonbuilder.h @@ -2537,6 +2537,7 @@ typedef struct J9StackWalkState { void* inlinedCallSite; void* stackMap; void* inlineMap; + UDATA loopBreaker; /* The size of J9StackWalkState must be a multiple of 8 because it is inlined into * J9VMThread where alignment assumotions are being made. */ diff --git a/runtime/vm/swalk.c b/runtime/vm/swalk.c index df83f21cd3c..c26cee0f930 100644 --- a/runtime/vm/swalk.c +++ b/runtime/vm/swalk.c @@ -91,9 +91,40 @@ static void dropToCurrentFrame (J9StackWalkState * walkState); /* The minimum number of stack slots that a stack frame can occupy */ #define J9_STACKWALK_MIN_FRAME_SLOTS (OMR_MIN(sizeof(J9JITFrame), sizeof(J9SFStackFrame)) / sizeof(UDATA)) +static VMINLINE UDATA +maxFramesOnStack(J9StackWalkState *walkState) +{ + J9VMThread *walkThread = walkState->walkThread; + J9JITConfig *jitConfig = walkThread->javaVM->jitConfig; + UDATA * endOfStack = walkThread->stackObject->end; + UDATA * sp = walkState->walkThread->sp; + UDATA maxFrames = (endOfStack - sp) / J9_STACKWALK_MIN_FRAME_SLOTS; + + if (NULL != jitConfig) { + if (J9_ARE_NO_BITS_SET(walkState->flags, J9_STACKWALK_SKIP_INLINES)) { + /* It's possible every frame on the stack is a fully inlined JIT frame */ + maxFrames *= (jitConfig->maxInlineDepth + 1); + } + } + + if (J9_ARE_NO_BITS_SET(walkState->flags, J9_STACKWALK_NO_ERROR_REPORT)) { + UDATA * stackStart = J9_LOWEST_STACK_SLOT(walkState->walkThread); +#if defined(J9VM_INTERP_STACKWALK_TRACING) + Assert_VRB_true(sp >= stackStart); + Assert_VRB_true(sp <= endOfStack); +#else /* J9VM_INTERP_STACKWALK_TRACING */ + Assert_VM_true(sp >= stackStart); + Assert_VM_true(sp <= endOfStack); +#endif /* J9VM_INTERP_STACKWALK_TRACING */ + } + + return maxFrames; +} + UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState) { - UDATA rc = (walkState->walkThread->privateFlags & J9_PRIVATE_FLAGS_STACK_CORRUPT) ? J9_STACKWALK_RC_STACK_CORRUPT : J9_STACKWALK_RC_NONE; + UDATA rc = J9_STACKWALK_RC_NONE; + UDATA walkRC = 0; J9Method * nextLiterals; UDATA * nextA0; UDATA savedFlags = walkState->flags; @@ -120,9 +151,15 @@ UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState) walkState->flags &= ~J9_STACKWALK_RESUME; goto resumeInterpreterWalk; } - if (currentThread != NULL) { + + walkState->loopBreaker = 0; + if (NULL != currentThread) { oldState = currentThread->activeWalkState; currentThread->activeWalkState = walkState; + if (J9_ARE_ANY_BITS_SET(currentThread->privateFlags2, J9_PRIVATE_FLAGS2_ASYNC_GET_CALL_TRACE)) { + /* Add one because walkFrames decrements loopBreaker and bails when it reaches 0 */ + walkState->loopBreaker = maxFramesOnStack(walkState) + 1; + } } walkState->javaVM = walkState->walkThread->javaVM; @@ -330,7 +367,9 @@ UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState) /* Walk the frame */ - if (walkFrame(walkState) != J9_STACKWALK_KEEP_ITERATING) { + walkRC = walkFrame(walkState); + if (J9_STACKWALK_KEEP_ITERATING != walkRC) { + rc = walkRC; goto terminationPoint; } resumeInterpreterWalk: @@ -342,7 +381,9 @@ UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState) #ifdef J9VM_INTERP_NATIVE_SUPPORT if (walkState->frameFlags & J9_STACK_FLAGS_JIT_TRANSITION_TO_INTERPRETER_MASK) { resumeJitWalk: - if (jitWalkStackFrames(walkState) != J9_STACKWALK_KEEP_ITERATING) { + walkRC = jitWalkStackFrames(walkState); + if (J9_STACKWALK_KEEP_ITERATING != walkRC) { + rc = walkRC; goto terminationPoint; } walkState->decompilationRecord = NULL; @@ -400,12 +441,23 @@ UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState) currentThread->activeWalkState = oldState; } + if (J9_ARE_ANY_BITS_SET(walkState->walkThread->privateFlags, J9_PRIVATE_FLAGS_STACK_CORRUPT)) { + rc = J9_STACKWALK_RC_STACK_CORRUPT; + } + return rc; } UDATA walkFrame(J9StackWalkState * walkState) { + if (0 != walkState->loopBreaker) { + walkState->loopBreaker -= 1; + if (0 == walkState->loopBreaker) { + return J9_STACKWALK_RC_STACK_CORRUPT; + } + } + if (walkState->flags & J9_STACKWALK_VISIBLE_ONLY) { if ((((UDATA) walkState->pc == J9SF_FRAME_TYPE_NATIVE_METHOD) || ((UDATA) walkState->pc == J9SF_FRAME_TYPE_JNI_NATIVE_METHOD)) && !(walkState->flags & J9_STACKWALK_INCLUDE_NATIVES)) { @@ -519,38 +571,17 @@ UDATA walkFrame(J9StackWalkState * walkState) static UDATA allocateCache(J9StackWalkState * walkState) { PORT_ACCESS_FROM_WALKSTATE(walkState); - UDATA * endOfStack = walkState->walkThread->stackObject->end; - UDATA framesPresent = 0; + UDATA framesPresent = maxFramesOnStack(walkState); UDATA cacheElementSize = 0; UDATA cacheSize = 0; UDATA * stackStart = J9_LOWEST_STACK_SLOT(walkState->walkThread); UDATA * sp = walkState->walkThread->sp; - if (J9_ARE_NO_BITS_SET(walkState->flags, J9_STACKWALK_NO_ERROR_REPORT)) { -#if defined(J9VM_INTERP_STACKWALK_TRACING) - Assert_VRB_true(sp >= stackStart); - Assert_VRB_true(sp <= endOfStack); -#else /* J9VM_INTERP_STACKWALK_TRACING */ - Assert_VM_true(sp >= stackStart); - Assert_VM_true(sp <= endOfStack); -#endif /* J9VM_INTERP_STACKWALK_TRACING */ - } - - framesPresent = (endOfStack - sp) / J9_STACKWALK_MIN_FRAME_SLOTS; - if (walkState->flags & J9_STACKWALK_CACHE_PCS) ++cacheElementSize; if (walkState->flags & J9_STACKWALK_CACHE_CPS) ++cacheElementSize; if (walkState->flags & J9_STACKWALK_CACHE_METHODS) ++cacheElementSize; cacheSize = framesPresent * cacheElementSize; -#ifdef J9VM_INTERP_NATIVE_SUPPORT - if (walkState->walkThread->javaVM->jitConfig) { - if (!(walkState->flags & J9_STACKWALK_SKIP_INLINES)) { - /* computations above assume 1 cacheElement per frame, in reality there may be (maxInlinedMethods + 1) elements per frame */ - cacheSize *= (walkState->walkThread->javaVM->jitConfig->maxInlineDepth + 1); - } - } -#endif if ((walkState != walkState->walkThread->stackWalkState) || ((UDATA) (sp - stackStart) < cacheSize) #if defined (J9VM_INTERP_VERBOSE) || defined (J9VM_PROF_EVENT_REPORTING)