Right :)

Thanks again for the review!

/peter

> On 22 maj 2014, at 01:10, David Holmes <david.hol...@oracle.com> wrote:
> 
>> On 21/05/2014 11:01 PM, Peter Allwin wrote:
>> D’oh! Great catch, I’ll update before the push if that’s OK.
> 
> Sure - I assume it is the comment that will be updated :)
> 
> Thanks,
> David
> 
>> 
>> Thanks,
>> 
>> /peter
>> 
>>> On 21 May 2014, at 13:32, David Holmes <david.hol...@oracle.com> wrote:
>>> 
>>> Hi Peter,
>>> 
>>> One inconsistency
>>> 
>>> 27  * Test to verify GetObjectSize does not overflow on a 600K element int[]
>>> 
>>> but
>>> 
>>>  34         int[] a = new int[600_000_000];
>>> 
>>> looks more like 600M
>>> 
>>> David
>>> 
>>>> On 21/05/2014 9:10 PM, Peter Allwin wrote:
>>>> Thanks Leonid, Serguei and David for your reviews!
>>>> 
>>>> Updated webrev is here: 
>>>> http://cr.openjdk.java.net/~allwin/8027230/webrev.01
>>>> 
>>>> Changes:
>>>> 
>>>>    - Agent process is now started trough ProcessBuilder
>>>>    - Non 64bit platforms are immediately skipped
>>>>    - Spacing before if/catch
>>>>    - Test TEST.groups updated
>>>>        Need compact3:
>>>>            serviceability/jvmti/GetObjectSizeOverflow.java (uses 
>>>> java.lang.Instrument)
>>>>            serviceability/jvmti/TestRedefineWithUnresolvedClass.java (uses 
>>>> java.lang.Instrument)
>>>>        Need JDK
>>>>            serviceability/jvmti/8036666/GetObjectLockCount.java (used 
>>>> com.sun.jdi)
>>>> 
>>>> Thanks!
>>>> /peter
>>>> 
>>>> 
>>>>> On 21 May 2014, at 07:54, David Holmes <david.hol...@oracle.com> wrote:
>>>>> 
>>>>>> On 21/05/2014 1:19 AM, Leonid Mesnik wrote:
>>>>>> Peter
>>>>>> 
>>>>>>   35  * @run main/othervm -Xmx4000m -javaagent:agent.jar
>>>>>> GetObjectSizeOverflowAgent
>>>>>> 
>>>>>> I think that "-Xmx4000m" cause test failure for 32-bit VM. It would be
>>>>>> better to use another 1 process builder.
>>>>> 
>>>>> If you need a 4GB heap to test this you will have to limit it to 64-bit 
>>>>> platforms.
>>>>> 
>>>>>> Also I think you need to add your test into need_jdk because you use
>>>>>> jar. Could you please check this with embedded team.
>>>>> 
>>>>> JDKToolFinder should get jar from the compile JDK rather than the test 
>>>>> JDK so that should be okay.
>>>>> 
>>>>> However the use of the agent/instrumentation is limited to compact3 
>>>>> profile (if java.lang.instrument is used) and not the minimal VM, so 
>>>>> changes are needed in TEST.groups. It looks like we have a number of 
>>>>> missing updates to the groups file for the  test/serviceability/jvmti 
>>>>> tests.
>>>>> 
>>>>> David
>>>>> -----
>>>>> 
>>>>> 
>>>>>> Leonid
>>>>>> 
>>>>>>> On 20.05.2014 19:02, Peter Allwin wrote:
>>>>>>> Hello!
>>>>>>> 
>>>>>>> Please review this simple fix for an integer overflow in JVMTI
>>>>>>> GetObjectSize().
>>>>>>> 
>>>>>>> webrev: http://cr.openjdk.java.net/~allwin/8027230/webrev.00/
>>>>>>> cr: https://bugs.openjdk.java.net/browse/JDK-8027230
>>>>>>> 
>>>>>>> 
>>>>>>> Testing:
>>>>>>>    New regression test
>>>>>>>    nsk.quick-jvmti.testlist
>>>>>>> 
>>>>>>> Thanks!
>>>>>>> /peter
>> 

Reply via email to