Hi, thanks, apart from adding a setter for R13 in X86Frame, the other half
of the fix is this:
public Frame getCurrentFrameGuess(JavaThread thread, Address addr) {
ThreadProxy t = getThreadProxy(addr);
AMD64ThreadContext context = (AMD64ThreadContext) t.getContext();
AMD64CurrentFrameGuess guesser = new AMD64CurrentFrameGuess(context,
thread);
if (!guesser.run(GUESS_SCAN_RANGE)) {
return null;
}
if (guesser.getPC() == null) {
return new X86Frame(guesser.getSP(), guesser.getFP());
} else if (VM.getVM().getInterpreter().contains(guesser.getPC())) {
// pass the value of R13 which contains the bcp for the top level
frame
Address r13 = context.getRegisterAsAddress(AMD64ThreadContext.R13);
X86Frame frame = new X86Frame(guesser.getSP(), guesser.getFP(),
guesser.getPC());
frame.setR13(r13);
return frame;
} else {
return new X86Frame(guesser.getSP(), guesser.getFP(),
guesser.getPC());
}
}
(the whole "if pc in interpreter" block is new)
Overhead likely to be low as this is only used in diagnostic code. Can't
think of any risk but I'm not an expert on this code.
Cheers,
David
On Wed, 21 Nov 2018 at 19:01, JC Beyler <jcbey...@google.com> wrote:
> Hi David,
>
> I think the easiest would be to see whole change to understand the
> repercussions of the change. I would imagine that any change that helps
> stacktraces being more precise is a good thing but there are questions that
> arise every time:
> - What is the overhead of adding this?
> - Does this add any risk of failure?
>
> I'd imagine that the change is relatively small and should be easy to
> assess this but seeing it would make things easier to determine.
>
> That being said, I'm not a reviewer for OpenJDK so this is really just my
> 2cents,
> Jc
>
> On Wed, Nov 21, 2018 at 9:17 AM David Griffiths <david.griffi...@gmail.com>
> wrote:
>
>> Hi, I'm new to this mailing list and working on a project that makes use
>> of the SA classes to get stack traces from a paused in flight JVM (we can't
>> use JDWP). I have observed that if the top frame is in the interpreter it
>> reports the BCI and line number incorrectly. This is because
>> X86Frame.getInterpreterFrameBCI uses the value stored on the stack rather
>> than the actual live value stored in R13.
>>
>> I have a patch for this which lets
>> LinuxAMD64JavaThreadPDAccess.getCurrentFrameGuess pass the R13 value to
>> X86Frame so that the latter can then do:
>>
>> public int getInterpreterFrameBCI() {
>> Address bcp = addressOfInterpreterFrameBCX().getAddressAt(0);
>> // If we are in the top level frame then R13 may have been set for us
>> which contains
>> // the BCP. If so then let it take priority. If we are in a top level
>> interpreter frame,
>> // the BCP is live in R13 (on x86) and not saved in the BCX stack
>> slot.
>> if (r13 != null) {
>> bcp = r13;
>> }
>> Address methodHandle =
>> addressOfInterpreterFrameMethod().getAddressAt(0);
>>
>> and this fixes the problem.
>>
>> Does this sound like a good idea and if so should I submit a patch?
>>
>> Cheers,
>>
>> David
>>
>
>
> --
>
> Thanks,
> Jc
>
