Hi JSR292 folks,
This bug has been assigned to me because of 7120468 but it was not introduced by my changes. More exactly, my changes have cause an existing a bug to exhibit a different behavior, easier to catch by the QA.
In short, the simplest fix is to stop dumping last_sp, base_sp, saved_bp
and everything depending on them since they were false and were not fixed by my change.
Unless you just want me to remove all this lines and putback, someone else should take ownership of the bug. In fact, if you'd like the fix soon, a volunteer would be welcome even in that case because I have other priorities right now.
If you look at previous output, you will see that saved_bp was not correct before my fix (and that the stack_size was always 0):
MH invokespecial rcx_mh=0xe47c9d98 sp=(0xfc61dc1c+-4) stack_size=0 bp=0xfc61dbe0
MH invokeExact rcx_mh=0xe47ceb40 sp=(0xfc61dc1c+-4) stack_size=0 bp=0xfc61dbe0
MH adapter_filter/S0/ref rcx_mh=0xe47ceb40 sp=(0xfc61dc1c+-4) stack_size=0 bp=0xfc61dbe0
MH adapter_retype_only rcx_mh=0xe47caae8 sp=(0xfc61dbe8+-4) stack_size=0 bp=0xfc61dbac
MH invokestatic rcx_mh=0xe47caad0 sp=(0xfc61dbe8+-4) stack_size=0 bp=0xfc61dbac
MH return/ricochet_blob.bounce rcx=0xfc61dbb0 sp=(0xfc61dbec+-12962799) stack_size=0 bp=0xfc61dbb0
That wrong saved_bp is/was used to compute a bad last_sp and a bad base_sp. I did not change that code.
The real bug is that:
- saved_bp is still false
- the computation of last_sp and base_sp is not so trivial, even
if saved_bp was really the BP of the caller frame (see for
instance the use of BP in compiled frames).
My changes cause saved_bp to be slightly different. Previously, it was SP after several pushes done in trace_method_handle and an enter call. I moved the enter earlier to create a proper frame. This means that saved_bp is now the same as entry sp, instead of pointing into the new frame. Unfortunately, when mapping a RicochetFrame at the old (bad) saved_bp address, the old code was always finding a stack address (probably saved_regs but I did not check the exact slot). Hence, the test
always failed, causing base_sp not to be updated and stack_size to be 0.
With my change, rfp->saved_args_base() now falls into the saved registers, which has more random values. Hence, the previous test sometimes succeeds, causing a bad base_sp to be used and thus arbitrary stack sizes instead of 0. That different erroneous behavior was easier to catch by the QA :-)
As stated above, If I were to do the fix, I would just remove the printing of last_sp, base_sp and stack_size that can be wrong (and hence confusing) in some cases. To get more information, one can set +Verbose and dump the caller frame in a safe manner. The good news is that my new code no longer depends on saved_bp, last_sp and base_sp when dumping verbose information :-)
I would also stop dumping saved_bp. I will not try to fix saved_bp because that would need to be tested and I do not have time for that now :-)
If someone thinks we should not just stop printing this bad info and would instead prefer to have it fixed, he should volunteer for the bug :-)