Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear resolveFrameFlags after each frame walk #2569

Merged
merged 1 commit into from
Aug 9, 2018
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion runtime/vm/swalk.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,9 @@ UDATA walkStackFrames(J9VMThread *currentThread, J9StackWalkState *walkState)
if (walkFrame(walkState) != J9_STACKWALK_KEEP_ITERATING) {
goto terminationPoint;
}
walkState->previousFrameFlags = walkState->frameFlags;
resumeInterpreterWalk:
walkState->previousFrameFlags = walkState->frameFlags;
walkState->resolveFrameFlags = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intention that the resolveFrameFlags are only set if the current frame is a resolveFrame? And otherwise cleared?

Overall, I have to wonder why we bother with a separate resolveFrameFlags in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - a lot of this is historical cruft, but fixing that is for a different day.

resolveFrameFlags is set when the previously-walked frame is a resolve frame. Generally only applicable if the current frame is a JIT frame, but this seemed the safest and most "correct" way to fix it, since there clearly is one case where the resolve frame is not on top of a JIT frame.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for confirming that I understood the fix :)


/* Call the JIT walker if the current frame is a transition from the JIT to the interpreter */

Expand Down