Review request for 8058322: Zero name_index item of MethodParameters attribute cause MalformedParameterException

[Eric McCorkle]

Thank you.

The tests for this patch and the other hotspot patch that is currently
under review (both deal with parameter reflection) are here:

http://cr.openjdk.java.net/~emc/8062556/

Note that the rules forbid putting binaries in the jdk repo, so in order
to test bad class files, we have to put the data in java files.

On 10/30/14 18:54, David Holmes wrote:
> On 31/10/2014 3:19 AM, Eric McCorkle wrote:
>> I forgot to mention, there is a test for this change, but it has to be
>> added to the jdk repo.  Thus, I will add it after the change propagates.
> 
> No you can push the test (once it is reviewed) and the hotspot changes
> together into the team forest. You have to do full JDK builds via JPRT
> these days (use -testset hotspot).
> 
> PS. Code change reviewed, so if you add the test to the webrev we can
> review that too.
> 
> Thanks,
> David
> 
>>   I have created the following issue to track this:
>> https://bugs.openjdk.java.net/browse/JDK-8062556
>>
>> On 10/30/14 05:50, David Holmes wrote:
>>> May as well try to review this now ...
>>>
>>> Erik, given:
>>>
>>>   814     name = NULL;
>>>   815   }
>>>   816
>>>   817   Handle rh = java_lang_reflect_Parameter::create(CHECK_NULL);
>>>   818   java_lang_reflect_Parameter::set_name(rh(), name());
>>>
>>> How is name() handled when name is NULL? Is it some implicit conversion
>>> operator at play?
>>>
>>> Thanks,
>>> David
>>>
>>> On 30/10/2014 7:44 PM, David Holmes wrote:
>>>> On 30/10/2014 6:39 PM, Albert Noll wrote:
>>>>> On 10/29/2014 06:00 PM, Eric McCorkle wrote:
>>>>>> I need a second for hotspot changes, if I recall?
>>>>> For simple changes, 1 review is enough.
>>>>
>>>> Runtime requires 2 reviewers - can't speak for other teams. "simple" is
>>>> too subjective.
>>>>
>>>> David
>>>>
>>>>> Best,
>>>>> Albert
>>>>>
>>>>>> On 10/29/14 12:20, Coleen Phillimore wrote:
>>>>>>> Looks good, Eric.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>> On 10/29/14, 10:59 AM, Eric McCorkle wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Please review this simple change which addresses a failure
>>>>>>>> condition in
>>>>>>>> the method parameter reflection implementation.  In the initial
>>>>>>>> implementation of method parameter reflection, a parameter with a
>>>>>>>> parameter_name index of 0 denoted a parameter with no name, and
>>>>>>>> the VM
>>>>>>>> translated this into the empty string when creating the Parameter
>>>>>>>> object
>>>>>>>> to return to Java code.  However, towards the end of the 8
>>>>>>>> cycle, the
>>>>>>>> spec was updated to state that a zero parameter_name index should
>>>>>>>> denote
>>>>>>>> a parameter with no name, and should result in Parameter.getName()
>>>>>>>> returning an empty string, whereas the empty string /constant/ is
>>>>>>>> expressly forbidden as a parameter name, and should result in
>>>>>>>> MalformedParametersException.  The reflection API was updated to
>>>>>>>> reflect
>>>>>>>> this behavior, but it seems the VM still translates a
>>>>>>>> parameter_name
>>>>>>>> index of 0 into the empty string.  This patch removes that,
>>>>>>>> resulting in
>>>>>>>> correct behavior of the reflection API for parameters with no name.
>>>>>>>>
>>>>>>>> The webrev is here:
>>>>>>>> http://cr.openjdk.java.net/~emc/8058322/
>>>>>>>>
>>>>>>>> The bug is here:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8058322
>>>>>