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

Wang, Yang yang02.wang at sap.com
Thu Dec 15 02:46:08 PST 2011 

Hi Bertrand,
   I don't want to insert comments on the long text, so here are some points I would like to clarify:
1. I understand, hotspot does not use callee-saved register in compiled java code for performance reasons. However, ebp is an exception. It is pushed in the beginning of a function(Prolog), and poped in the end(Epilog). So I don't see any difference to the callee-save concept. If there's any difference(maybe from register allocator's perspective?), could you help clarify?

2. You mentioned "the GC should not have to update any OOP in callee-saved registers except arguments". However, the location of ebp is always updated explicitly  in frame traversal for the sake of GC(see function update_map_with_saved_link).

3. As SAP JVM is a server VM, all I mentioned is only about the server edition. I noticed there's difference between C1 and C2 in the context(see function sender_for_interpreter_frame). It may add extra difficulty to your review :(

Thanks,
Yang

-----Original Message-----
From: yang02.wang at sap.com 
Sent: Mittwoch, 14. Dezember 2011 18:55
To: 'Bertrand Delsart'
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: RE: Request for Review(S): JVM crashes when throwing StackOverflow exception from interpreter

Hello again,
Well, I think I am still not clear enough about the term "callee-save". From Java ABI's perspective, there is no callee-saved registers; but from C ABI's perspective, ebp is callee-saved, and it is causing problem : it saves and restores it without interacting with GC.

And I don't understand why you say " For instance, on x86, "ebp" cannot be an "oop that expects to be GCed" when at a method call site". Actually I saw "ebp" in OopMap in a call site with "PrintAssembly".

Thanks,
Yang
-----Original Message-----
From: Bertrand Delsart [mailto:bertrand.delsart at oracle.com] 
Sent: Mittwoch, 14. Dezember 2011 17:50
To: Wang, Yang
Cc: Volker Simonis; hotspot-compiler-dev at openjdk.java.net
Subject: Re: Request for Review(S): JVM crashes when throwing StackOverflow exception from interpreter

Sorry,

I just realized that x86 was not using the same stub.

I got caught up in the confusion created by the comments and did not 
notice the difference between the call to 
Interpreter::throw_StackOverflowError_entry() and the call to 
SharedRuntime::throw_StackOverflowError_entry()

The real reason for the failure seems to be that x86 was calling 
something that assumes the active frame is interpreted (see 
generate_StackOverflowError_handler). If this is called with a compiled 
frame context (after a c2i), this could indeed lead to issues that I 
need to investigate further.

I know understand better the reasons for your fix and will see whether 
this is indeed calling the right stub and whether I could generate it 
earlier to avoid delaying the load.

Thanks,

Bertrand.

On 12/14/11 05:14 PM, Bertrand Delsart wrote:
> 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

More information about the hotspot-compiler-dev mailing list