Review request: JDK-8062556: Add jdk tests for JDK-8058322 and JDK-8058313
Eric McCorkle
eric.mccorkle at oracle.com
Thu Nov 6 18:00:10 UTC 2014
Are there any concerns about the tests, other than the broken webrevs?
On 11/03/14 10:42, Eric McCorkle wrote:
> I have been having issues with webrev, which I reported earlier. Webrev
> reports a syntax error when I try to use it, and curiously, it fails to
> produce top-level files in this case (and this case only, as evidenced
> by my other webrevs).
>
> Unfortunately, there's nothing I can do about the missing top-level
> files; however, you can still look at the individual files just fine.
>
> On 11/02/14 23:50, David Holmes wrote:
>> Hi Erik,
>>
>> webrevs still broken for some reason.
>> On 1/11/2014 12:01 AM, Eric McCorkle wrote:
>>> I went through and added comments in the binary data indicating where
>>> the MethodParameters attributes are, and a breakdown of their contents.
>>> I went ahead and did this for all the bad class files, not just the new
>>> ones.
>>>
>>> There is a larger picture here: there's an outstanding task I filed
>>> around the time these tests were written to find a better way for
>>> langtools to run jtreg tests that involve bad class files.
>>> Unfortunately, doing that is rather difficult, as you can see. The only
>>> real way to do it is to generate a class file, convert it to signed
>>> bytes (you can't even use hex; you get an unsigned/signed byte
>>> conversion problem), then modify the data by hand. The intent is to
>>> replace this with a better method at some point.
>>
>> OK. New comments an improvement.
>>
>> Please give the new test the correct initial copyright year of 2014. I
>> know updates to the year are handled automatically (eventually) but we
>> should at least have things correct to start with.
>>
>> Thanks,
>> David
>>
>>> On 10/30/14 21:59, David Holmes wrote:
>>>> Hi Erik,
>>>>
>>>> On 31/10/2014 9:41 AM, Eric McCorkle wrote:
>>>>> Hello,
>>>>>
>>>>> Please review this patch which adds tests to the JDK test suite for two
>>>>> reflection bugs that require hotspot changes (JDK-8058322 and
>>>>> JDK-8058313)
>>>>>
>>>>> The webrev is here:
>>>>> http://cr.openjdk.java.net/~emc/8062556/
>>>>
>>>> I second Brian's comment re the source of the bad classes.
>>>>
>>>> Your webrev is broken btw - no top-level html files.
>>>>
>>>> The new test needs a copyright year of 2014 not 2013.
>>>>
>>>> Thanks,
>>>> David
>>>>
More information about the hotspot-dev
mailing list