Webrev updated to address these issues.
On 09/24/13 07:51, Joel Borggren-Franck wrote:
> 364 try {
> 365 tmp = getParameters0();
> 366 } catch(IllegalArgumentException e) {
> 367 // Rethrow ClassFormatErrors
> 368 throw new MalformedParametersException("Invalid constant
> pool index");
> 369 }
>
> What is causing the IAE? Have you considered having
> MalformedParametersExcepton taking a Throwable cause and having the IAE
> as cause?
>
The IAE comes from hotspot itself. There is a standard constant pool
index verifying function that throws IAE if given a bad index.
> On 2013-09-19, Eric McCorkle wrote:
>> The webrev has been updated with Joe's comments addressed.
>>
>> On 09/19/13 00:11, David Holmes wrote:
>>> On 19/09/2013 9:59 AM, Eric McCorkle wrote:
>>>> This still needs to be reviewed.
>>>
>>> You seem to have ignored Joe's comments regarding the change to Modifier
>>> being incorrect.
>>>
>>> David
>>>
>>>> On 09/16/13 14:50, Eric McCorkle wrote:
>>>>> I pulled the class files into byte array constants, as a temporary
>>>>> measure until a viable method for testing bad class files is developed.
>>>>>
>>>>> The webrev has been refreshed. The class files will be taken out before
>>>>> I push.
>>>>>
>>>>> http://cr.openjdk.java.net/~emc/8020981/
>>>>>
>>>>> On 09/13/13 20:48, Joe Darcy wrote:
>>>>>> To avoid storing binaries in Hg, you could try something like:
>>>>>>
>>>>>> * uuencode / ascii armor the class file
>>>>>> * convert to byte array in the test
>>>>>> * load classes from byte array
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>> On 09/13/2013 11:54 AM, Eric McCorkle wrote:
>>>>>>> I did it by hand with emacs.
>>>>>>>
>>>>>>> I would really rather tackle the bad class files for testing issue
>>>>>>> once
>>>>>>> and for all, the Right Way (tm). But with ZBB looming, now is not the
>>>>>>> time to do it.
>>>>>>>
>>>>>>> Hence, I have created this task
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8024674
>>>>>>>
>>>>>>> I also just created this one:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8024812
>>>>>>>
>>>>>>> On 09/13/13 13:54, Peter Levart wrote:
>>>>>>>> Hi Eric,
>>>>>>>>
>>>>>>>> How did you create those class files? By hand using a HEX editor? Did
>>>>>>>> you create a program that patched the original class file? If the
>>>>>>>> later
>>>>>>>> is the case, you could pack that patching logic inside a custom
>>>>>>>> ClassLoader...
>>>>>>>>
>>>>>>>> To hacky? Dependent on future changes of javac? At least the "bad
>>>>>>>> name"
>>>>>>>> patching could be performed trivially and reliably, I think:
>>>>>>>> search and
>>>>>>>> replace with same-length string.
>>>>>>>>
>>>>>>>> Regards, Peter
>>>>>>>>
>>>>>>>> On 09/13/2013 07:35 PM, Eric McCorkle wrote:
>>>>>>>>> Ugh. Byte arrays of class file data is really a horrible solution.
>>>>>>>>>
>>>>>>>>> I have already filed a task for test development post ZBB to
>>>>>>>>> develop a
>>>>>>>>> solution for generating bad class files. I'd prefer to file a
>>>>>>>>> follow-up
>>>>>>>>> to this to add the bad class file tests when that's done.
>>>>>>>>>
>>>>>>>>> On 09/13/13 10:55, Joel Borggrén-Franck wrote:
>>>>>>>>>> I think the right thing to do is to include the original compiling
>>>>>>>>>> source in a comment, together with a comment on how you modify
>>>>>>>>>> them, and then the result as a byte array.
>>>>>>>>>>
>>>>>>>>>> IIRC I have seen test of that kind before somewhere in our repo.
>>>>>>>>>>
>>>>>>>>>> cheers
>>>>>>>>>> /Joel
>>>>>>>>>>
>>>>>>>>>> On Sep 13, 2013, at 4:49 PM, Eric McCorkle
>>>>>>>>>> <eric.mccor...@oracle.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> There is no simple means of generating bad class files for
>>>>>>>>>>> testing.
>>>>>>>>>>> This is a huge deficiency in our testing abilities.
>>>>>>>>>>>
>>>>>>>>>>> If these class files shouldn't go in, then I'm left with no choice
>>>>>>>>>>> but
>>>>>>>>>>> to check in no test for this patch.
>>>>>>>>>>>
>>>>>>>>>>> However, anyone can run the test I've provided with the class
>>>>>>>>>>> files and
>>>>>>>>>>> see that it works.
>>>>>>>>>>>
>>>>>>>>>>> On 09/13/13 09:55, Joel Borggrén-Franck wrote:
>>>>>>>>>>>> Hi Eric,
>>>>>>>>>>>>
>>>>>>>>>>>> IIRC we don't check in classfiles into the repo.
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not sure how we handle testing of broken class-files in jdk,
>>>>>>>>>>>> but ASM might be an option, or storing the class file as an
>>>>>>>>>>>> embedded byte array in the test.
>>>>>>>>>>>>
>>>>>>>>>>>> cheers
>>>>>>>>>>>> /Joel
>>>>>>>>>>>>
>>>>>>>>>>>> On Sep 13, 2013, at 3:40 PM, Eric McCorkle
>>>>>>>>>>>> <eric.mccor...@oracle.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> A new webrev is posted (and crucible updated), which actually
>>>>>>>>>>>>> validates
>>>>>>>>>>>>> parameter names correctly. Apologies for the last one.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 09/12/13 16:02, Eric McCorkle wrote:
>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please review this patch, which implements correct behavior for
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> Parameter Reflection API in the case of malformed class files.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The bug report is here:
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8020981
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The webrev is here:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~emc/8020981/
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This review is also on crucible. The ID is CR-JDK8TL-182.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Eric
>>>>>>>>>>>>>>
>>>>>>>>>>> <eric_mccorkle.vcf>
>>>>>>
>
