Round 2 Code Review request for new MallocMaxTestWords option and fixed test (7030610, 7037122, 7123945)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Mar 19 09:39:05 PDT 2013
Just to close the loop on this part of the thread.
I wrote the following C test program:
$ cat overflow.c
#include <stdio.h>
main(int argc, char *argv[]) {
unsigned int at_limit = 0x7fffffff;
int a_little_over = at_limit + 10;
unsigned int over_limit = at_limit + 20;
printf("Hello from overflow!\n");
printf("\n");
printf("at_limit contains the largest signed int value ");
printf("in an unsigned int container.\n");
printf("sizeof(at_limit)=%d\n", (int) sizeof(at_limit));
printf("at_limit=%d (%%d format)\n", at_limit);
printf("at_limit=%u (%%u format)\n", (unsigned int) at_limit);
printf("\n");
printf("a_little_over contains (at_limit + 10) value ");
printf("in a signed int container.\n");
printf("sizeof(a_little_over)=%d\n", (int) sizeof(a_little_over));
printf("a_little_over=%d (%%d format)\n", a_little_over);
printf("a_little_over=%u (%%u format)\n", (unsigned int)
a_little_over);
printf("\n");
if (a_little_over > at_limit) {
printf("if-statement says '(a_little_over > at_limit) == true'\n");
printf("This result means that overflow did not happen.\n");
} else {
printf("if-statement says '(a_little_over > at_limit) == false\n");
printf("This result means that overflow happened.\n");
}
printf("\n");
if ((at_limit + 5) > at_limit) {
printf("if-statement says '((at_limit + 5) > at_limit) ==
true'\n");
printf("This result means that overflow did not happen.\n");
} else {
printf("if-statement says '((at_limit + 5) > at_limit) ==
false\n");
printf("This result means that overflow happened.\n");
}
printf("\n");
printf("over_limit contains (at_limit + 20) value ");
printf("in an unsigned int container.\n");
printf("sizeof(over_limit)=%d\n", (int) sizeof(over_limit));
printf("over_limit=%d (%%d format)\n", over_limit);
printf("over_limit=%u (%%u format)\n", (unsigned int) over_limit);
printf("\n");
if (a_little_over > over_limit) {
printf("if-statement says '(a_little_over > over_limit) ==
true'\n");
printf("This result means that overflow happened.\n");
} else {
printf("if-statement says '(a_little_over > over_limit) ==
false\n");
printf("This result means that overflow did not happen.\n");
}
printf("\n");
if ((a_little_over + 15) > over_limit) {
printf("if-statement says '(a_little_over+15 > over_limit) ==
true'\n");
printf("This result means that overflow did not happen.\n");
} else {
printf("if-statement says '(a_little_over+15 > over_limit) ==
false\n");
printf("This result means that overflow happened.\n");
}
printf("\n");
}
Here are the results for MacOS X:
$ cc --version
i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5666) (dot 3)
Copyright (C) 2007 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ ./overflow
Hello from overflow!
at_limit contains the largest signed int value in an unsigned int container.
sizeof(at_limit)=4
at_limit=2147483647 (%d format)
at_limit=2147483647 (%u format)
a_little_over contains (at_limit + 10) value in a signed int container.
sizeof(a_little_over)=4
a_little_over=-2147483639 (%d format)
a_little_over=2147483657 (%u format)
if-statement says '(a_little_over > at_limit) == true'
This result means that overflow did not happen.
if-statement says '((at_limit + 5) > at_limit) == true'
This result means that overflow did not happen.
over_limit contains (at_limit + 20) value in an unsigned int container.
sizeof(over_limit)=4
over_limit=-2147483629 (%d format)
over_limit=2147483667 (%u format)
if-statement says '(a_little_over > over_limit) == false
This result means that overflow did not happen.
if-statement says '(a_little_over+15 > over_limit) == true'
This result means that overflow did not happen.
Here are the results for Solaris X86:
$ cc -V
cc: Sun C 5.10 SunOS_i386 Patch 142363-05 2010/04/28
usage: cc [ options] files. Use 'cc -flags' for details
512 2013.03.19 10:37:52 $ ./overflow
Hello from overflow!
at_limit contains the largest signed int value in an unsigned int container.
sizeof(at_limit)=4
at_limit=2147483647 (%d format)
at_limit=2147483647 (%u format)
a_little_over contains (at_limit + 10) value in a signed int container.
sizeof(a_little_over)=4
a_little_over=-2147483639 (%d format)
a_little_over=2147483657 (%u format)
if-statement says '(a_little_over > at_limit) == true'
This result means that overflow did not happen.
if-statement says '((at_limit + 5) > at_limit) == true'
This result means that overflow did not happen.
over_limit contains (at_limit + 20) value in an unsigned int container.
sizeof(over_limit)=4
over_limit=-2147483629 (%d format)
over_limit=2147483667 (%u format)
if-statement says '(a_little_over > over_limit) == false
This result means that overflow did not happen.
if-statement says '(a_little_over+15 > over_limit) == true'
This result means that overflow did not happen.
And here are the results for WinXP:
$ cl
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.30319.01
for 80x86
Copyright (C) Microsoft Corporation. All rights reserved.
usage: cl [ option... ] filename... [ /link linkoption... ]
$ ./overflow
Hello from overflow!
at_limit contains the largest signed int value in an unsigned int container.
sizeof(at_limit)=4
at_limit=2147483647 (%d format)
at_limit=2147483647 (%u format)
a_little_over contains (at_limit + 10) value in a signed int container.
sizeof(a_little_over)=4
a_little_over=-2147483639 (%d format)
a_little_over=2147483657 (%u format)
if-statement says '(a_little_over > at_limit) == true'
This result means that overflow did not happen.
if-statement says '((at_limit + 5) > at_limit) == true'
This result means that overflow did not happen.
over_limit contains (at_limit + 20) value in an unsigned int container.
sizeof(over_limit)=4
over_limit=-2147483629 (%d format)
over_limit=2147483667 (%u format)
if-statement says '(a_little_over > over_limit) == false
This result means that overflow did not happen.
if-statement says '(a_little_over+15 > over_limit) == true'
This result means that overflow did not happen.
My test program might be flawed, but so far I don't
see any overflows happening.
Dan
On 3/18/13 8:39 PM, Daniel D. Daugherty wrote:
> Greetings,
>
> Now that it looks like the dust is settling on round 2 of
> this code review, I have a question for the code reviewers.
>
> This if-statement came up ina couple of reviews:
>
> 580 if (MallocMaxTestWords > 0 &&
> 581 (cur_malloc_words + (alloc_size / BytesPerWord)) >
> MallocMaxTestWords) {
>
> Nobody was worried about the part on line 580and I had
> a very simple way of looking at line 581:
>
> (expr1 > expr2)
>
> but it occurs to me that maybe I'm wrong. 'expr2' is
> pretty simple: MallocMaxTestWords which is of type
> uintx. As David H pointed out, uintx is 32-bits or
> 64-bits depending on the VM (and the OS). To me the
> important partwas not explicitly stated and that is
> the factthat uintx is unsigned.
>
> Since 'expr2' is unsigned, 'expr1' will be "promoted"
> by the compiler to unsigned for the comparison. In
> that case, "overflow"or "rollover" doesn't matter
> because we're comparing two unsigned values.
>
> So the question: Am I misunderstanding how 'unsigned'
> works in comparison expressions in C++/C?
>
> Perhaps I was looking at this way too simplistically.
>
> Dan
>
> P.S.
> Another way that I looked at these changesis risk:
>
> If Ron's early malloc() failure logic doesn'tcatch
> when the (artificial) limit is reached, thenwhat
> happens? We don't return NULL early and the system
> works the way it did before. Little to no risk.
>
>
> On 3/15/13 12:29 PM, Ron Durbin wrote:
>> 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/20130319/71487dc7/attachment-0001.html
More information about the hotspot-runtime-dev
mailing list