Round 2 Code Review request for new MallocMaxBytes option and fixed test (7030610, 7037122, 7123945)

[Ioi Lam]

Hi Ron,

Is it possible to change MallocMaxTestWords back MallocMaxTestBytes? The 
reasons are:

  * I am surprised that Coleen hasn't complained yet.
  * It would be good if you can say -XX:MallocMaxTestBytes=128M and
    won't be confused about what it means.
  * For the tests that need to use this option (for testing the handling
    of malloc failures), do they really need to allocate more than 2GB
    of stuff?
  * (alloc_size / BytesPerWord) will give you zero if the test case keep
    allocating 7 bytes or less (on 64-bit).

for overflow checks, you probably need

    if (int(old_bytes + new_bytes) < (int)(old_bytes) ||
         old_bytes + new_bytes > MallocMaxTestWords) {
         return NULL; // fail to alloc
    }

I suggest limiting MallocMaxTestWords to no more than 0x7fffffff so that 
the logic would be a little cleaner and easier to understood.

Maybe we should also add sizeof(int*) to the requested size, and round 
the size up to sizeof(int*)? This would be a good estimate for the 
malloc overhead.

- Ioi


On 03/15/2013 11:29 AM, Ron Durbin wrote:
>
> Dan
>
> I have a round 2 code review ready for:
>
> Round 2 addresses:
>
> - addressed code review comments from Coleen, Dan and David
> - note: src/share/vm/runtime/os.hpp is no longer changed.
>
> Web URL 
> http://cr.openjdk.java.net/~rdurbin/7030610-webrev-cr1/1-hsx25/ 
> <http://cr.openjdk.java.net/%7Erdurbin/7030610-webrev-cr1/1-hsx25/>
>
> Internal URL 
> http://javaweb.us.oracle.com/~rdurbin/7030610-webrev/1-hsx25/ 
> <http://javaweb.us.oracle.com/%7Erdurbin/7030610-webrev/1-hsx25/>
>
> 7030610 runtime/6878713/Test6878713.sh fails, Error. failed to clean 
> up files after test.
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7030610
>
> https://jbs.oracle.com/bugs/browse/JDK-7030610
>
> 7037122 runtime/6878713/Test6878713.sh is written incorrectly
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7037122
>
> https://jbs.oracle.com/bugs/browse/JDK-7037122
>
> (This fix only addresses the issues with test 
> runtime/6878713/Test6878713.sh;
>
> 7037122 names other tests that are not addressed by this fix)
>
> 7123945 runtime/6878713/Test6878713.sh require about 2G of
>
> _http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123945_
>
> https://jbs.oracle.com/bugs/browse/JDK-7123945
>
> Summary: Add new diagnostic option -XX:MallocMaxTestWords=NNN and fix
>
> Test6878713.sh. Test6878713.sh is a security regression test with four 
> different
> failure modes.Three are documented in the defects above and the forth
>
> was found during bug triage. The root cause of these failures can be
>
> traced back to two issues: Inconsistent test condition setup and error 
> handling.
>
> All four defect manifestations are fixed by this set of changes.
>
> The testing is not completed because the JPRT run is running slowly.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130318/10fbb23b/attachment-0001.html