1-line review request: 7194607 VerifyLocalVariableTableOnRetransformTest.sh fails after JSR-292 merge

[serguei.spitsyn at oracle.com]

Ok, it seems there are some suspicious fragments in the interpreter code.
Christian, could you, please, check and comment the fragments below?

This is how the Method::max_stack() is defined:

*src/share/vm/oops/method.hpp*:

   int  verifier_max_stack() const                { return _max_stack; }
   int           max_stack() const                { return _max_stack + 
extra_stack_entries(); }
   void      set_max_stack(int size)              { _max_stack = size; }
   . . .
   static int extra_stack_entries() { return EnableInvokeDynamic ? 2 : 0; }


The following code fragments are unaware that the method->max_stack() 
returns _max_stack + extra_stack_entries() :

*src/cpu/sparc/vm/cppInterpreter_sparc.cpp*:
*src/cpu/sparc/vm/cppInterpreter_***x86*.cpp*:

static int size_activation_helper(int callee_extra_locals, int 
max_stack, int monitor_size) {
   . . .
   const int extra_stack = 0; //6815692//Method::extra_stack_entries(); 
      ????
   return (round_to(max_stack +
                    extra_stack +
                    . . .
}
. . .
void BytecodeInterpreter::layout_interpreterState(interpreterState to_fill,
   . . .
   int extra_stack = 0; 
//6815692//Method::extra_stack_entries();               ????
   to_fill->_stack_limit = stack_base - (method->max_stack() + 1 + 
extra_stack);
   . . .
}


*src/cpu/sparc/vm/templateInterpreter_sparc.cpp*:

static int size_activation_helper(int callee_extra_locals, int 
max_stack, int monitor_size) {
   . . .
   const int max_stack_words = max_stack * Interpreter::stackElementWords;
   return (round_to((max_stack_words
                    //6815692//+ 
Method::extra_stack_words()                           ????
   . . .
}

At the size_activation_helper call sites the second parameter is usually 
passed as method->max_stack().


*src/cpu/x86/vm/templateInterpreter_x86_32.cpp*:
*src/cpu/x86/vm/templateInterpreter_x86_64.cpp*:

int AbstractInterpreter::size_top_interpreter_activation(Method* method) {
   . . .
   const int extra_stack = Method::extra_stack_entries();
   const int method_stack = (method->max_locals() + method->max_stack() 
+ extra_stack) *
                            Interpreter::stackElementWords;
   . . .
}


*src/share/vm/interpreter/oopMapCache.cpp*:

void OopMapForCacheEntry::compute_map(TRAPS) {
   . . .
   // First check if it is a method where the stackmap is always empty
   if (method()->code_size() == 0 || method()->max_locals() + 
method()->max_stack() == 0) {
     _entry->set_mask_size(0);
   } else {
   . . .
}

Above, if the invokedynamic is enabled then the method()->max_stack() 
can not be 0.
We need to check it if this fact does not break the fragment.


I'm still looking at other places...


Thanks,
Serguei


On 10/30/12 10:41 AM, serguei.spitsyn at oracle.com wrote:
> I have a plan to look at it, at least for other serviceablity code.
> It'd be good if someone from the runtime or compiler team checked it too.
>
> Thanks,
> Serguei
>
>
> On 10/30/12 10:37 AM, Daniel D. Daugherty wrote:
>> Thumbs up.
>>
>> Is someone going to do an audit for similar missing changes
>> from max_stack() (not max_size()) to verifier_max_stack()?
>>
>> Dan
>>
>>
>> On 10/30/12 1:30 PM, serguei.spitsyn at oracle.com wrote:
>>> Hello,
>>>
>>>
>>> Please, review the fix for CR:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7194607
>>>
>>> CR in JIRA:
>>> https://jbs.oracle.com/bugs/browse/JDK-7194607
>>>
>>> Open webrev:
>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7194607-JVMTI-max_size
>>>
>>>
>>> Summary:
>>>
>>> This issue is caused by the changes in the oops/method.hpp for 
>>> invokedynamic (JSR 292).
>>> Now the max_stack() adds +2 to the original code attribute stack 
>>> size if invokedynamic is enabled.
>>> The verifier_max_stack() must be used in the 
>>> jvmtiClassFileReconstituter.cpp
>>> instead of the max_size() to get the code attribute stack size.
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20121030/671fd34f/attachment.html