Request for Review(S): JVM crashes when throwing StackOverflow exception from interpreter

Wed Dec 14 08:14:43 PST 2011

Hi Yang,

Answers inlined.

On 12/14/11 03:26 PM, Wang, Yang wrote:
> 1. For the stub concern, it is true that there is an existing stub, but it is only invoked via implicit exception handler(see function continuation_for_implicit_exception). For explicit stack overflow check in "interpreter", the stub is in hotspot not used previously. So the fix is to borrow it. Since the stub is generated after the code-generation for interpreter, the entry address was load indirectly.

The stub was used on x86 in the call you modified :-) In addition, some 
stubs are generated before the interpreter. That's why I asked whether 
you had really noticed an issue (e.g. whether the code generated a jump 
to a NULL address on x86).

Anyway, I'll double check when the stub is generated and, if necessary, 
see whether we can initialize it earlier instead of deferring the load.

> 2. Going through the stub is like a guard for the VM call, as shown in the picture in webrev: stub frame on top of C frame. When GC fails to handle the OOP in C frame, the stub frame remedies it by containing a copy of the OOP, which is to be handled by GC.

Understood. The problem is not really about whether we need a stub (I'm 
still reviewing that for sparc). The problem is about the reasons 
stated. I'm trying to remove the comments that might be confusing.

For instance, on x86, "ebp" cannot be an "oop that expects to be GCed" 
when at a method call site (elsewhere, it can indeed be an oop that 
needs to be updated). See point 3. From a  jited code point of view, 
there is no callee-saved registers in our compiled Java ABI for x86.

I'm currently reviewing the Sparc part because it is indeed a special 
case... and the one that was unfortunately not using the stub :-(

> 3. For the concern of callee-saved register, I guess you mean Sparc only. You are right that the IN and Local registers are not called "callee-saved" technically(but "never-saved"). However, they are also saved by callee in callee's frame in certain cases(like register window overflow). I hope my understanding here is correct.

No. That was not sparc only. In fact Sparc might be the exception.

If you look for instance at the definition of is_caller_save_register() 
in c1_FrameMap_x86.hpp, you'll see that HotSpot now assumes that all 
registers can be scratched by called Java methods. We measured that 
performance were better when not using callee saved registers (for 
compiled Java code).

Sparc is a special case because the IN and Local registers are 
automatically protected. This is the only platform which provides 
callee-saved registers "for free" when building frames (thanks to the 
register windows). Hence, I thought that our JIT compilers were no 
longer using callee-saved registers on any of the supported platforms 
but Sparc might indeed be the exception.

>
> Thanks and welcome more inputs!
> Yang
>
>
>
> -----Original Message-----
> From: Bertrand Delsart [mailto:bertrand.delsart at oracle.com]
> Sent: Mittwoch, 14. Dezember 2011 13:36
> To: Wang, Yang
> Cc: Volker Simonis; hotspot-compiler-dev at openjdk.java.net; Coleen Phillimore
> Subject: Re: Request for Review(S): JVM crashes when throwing StackOverflow exception from interpreter
>
> Sorry,
>
> I was notified of the CR and got involved in the bug because we were
> wondering whether the bug was impacting our embedded ports. I sent my
> own review in an internal alias but should have let you know. FYI, we
> were still discussing it because I raised some issues about the proposed
> fix.
>
> First, thanks for spotting the bug! We do agree with the first problem.
> We should indeed ensure that SP is correct.
>
> My concerns were about the second part.
>
> First, the change is doing more that just going through the stub. It
> seems that your are trying to solve a third problem. x86 was already
> using the throw_StackOverflowError_entry stub. What your proposed change
> seems to imply is that you encountered a case where the stub had not yet
> been generated and hence you had to delay the loading of its address. Is
> that correct ?
>
> In addition, the explanations about why we have to go through that stub
> did not fit my knowledge of HotSpot and I was trying to clarify that
> internally (thanks Tom for your confirmations).
>
> With our Java ABI, none of the registers allocated by the compiler are
> callee-saved. Hence, at call sites, the GC should not have to update any
>    OOP in callee-saved registers (but might have to update arguments). If
> fixing SP is not sufficient to solve your test case on SPARC then the
> problem is not exactly what you describe.
>
> Now, that does not mean the fix would not be correct :-) Going through
> that stub might be safer anyway, particularly since we are doing it on
> x86. My issues are about the comments themselves (and the third problem
> mentioned above).
>
> Thanks,
>
> Bertrand.
>
> On 12/13/11 09:19 PM, Christian Thalinger wrote:
>>
>> On Dec 7, 2011, at 10:45 AM, Volker Simonis wrote:
>>
>>> There's now a bug report for this issue:
>>>
>>> 7116216: StackOverflow GC crash
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7116216
>>>
>>> so could somebody please have a look at the fix proposed by Yang?
>>
>> Sorry, I will have a look tomorrow.
>>
>> -- Chris
>>
>>>
>>> Regards,
>>> Volker
>>>
>>> On Wed, Nov 16, 2011 at 6:04 PM, Wang, Yang<yang02.wang at sap.com>   wrote:
>>>> Hi folks,
>>>>
>>>>      We found two problems during throwing an StackOverflow exception from
>>>> interpreter.
>>>>
>>>> 1.       Before preparing to throw a StackOverflow exception, the last Java
>>>> frame is set to the current sp. This is problematic when the StackOverflow
>>>> is thrown on top of c2i adapter. Solution : the real caller frame(unextended
>>>> sp) should be set as last Java frame.
>>>>
>>>> 2.       When Garbage collection happens during throwing StackOverflow
>>>> exception, and callee-saved register(or "never-saved" register which behaves
>>>> alike) happens to be an OOP, GC is unable to locate the OOP in C
>>>> frame(generated by VM calls), and hence fails to process the OOP. Solution :
>>>> We build a runtime stub frame before doing a VM call, which guarantees the
>>>> location of Callee-saved registers are always recognizable by GC.
>>>>
>>>>      Detailed descriptions and tests could be found in webrev
>>>>    http://www.sapjvm.com/yw/webrevs/StackOverflow_GC_Crash/
>>>>
>>>>      I don't have a bugID yet. Please kindly open one bug for this issue.
>>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yang
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Yang Wang
>>>> Software Engineer
>>>>
>>>> TIP Core AS&VM Technology (AG)
>>>> SAP JVM JIT Compiler
>>>>
>>>> SAP AG
>>>> Dietmar-Hopp-Allee 16
>>>> 69190 Walldorf, Germany
>>>>
>>>> T +49 6227 7-50320
>>>> F +49 6227 78-48541
>>>> Email : yang02.wang at sap.com
>>>> www.sap.com
>>>>
>>>>
>>>> Pflichtangaben/Mandatory Disclosure Statements:
>>>> http://www.sap.com/company/legal/impressum.epx
>>>>
>>>> Diese E-Mail kann Betriebs- oder Geschäftsgeheimnisse oder sonstige
>>>> vertrauliche Informationen enthalten. Sollten Sie diese E-Mail irrtümlich
>>>> erhalten haben, ist Ihnen eine Kenntnisnahme des Inhalts, eine
>>>> Vervielfältigung oder Weitergabe der E-Mail ausdrücklich untersagt. Bitte
>>>> benachrichtigen Sie uns und vernichten Sie die empfangene E-Mail. Vielen
>>>> Dank.
>>>>
>>>> This e-mail may contain trade secrets or privileged, undisclosed, or
>>>> otherwise confidential information. If you have received this e-mail in
>>>> error, you are hereby notified that any review, copying, or distribution of
>>>> it is strictly prohibited. Please inform us immediately and destroy the
>>>> original transmittal. Thank you for your cooperation.
>>>>
>>>>
>>
>
>

-- 
Bertrand Delsart,               bertrand.delsart at oracle.com,
Sun-Oracle France, 180 av. de l'Europe, ZIRST de Montbonnot,
38334 Saint Ismier,                                  FRANCE