On Wed, 16 Mar 2022 17:45:16 GMT, Tyler Steele <d...@openjdk.java.net> wrote:
>> As described in the linked issue, NullClassBytesTest fails due an >> OutOfMemoryError produced on AIX when the test calls defineClass with a byte >> array of size of 0. The native implementation of defineClass then calls >> malloc with a size of 0. On AIX malloc(0) returns NULL, while on other >> platforms it return a valid address. When NULL is produced by malloc for >> this reason, ClassLoader.c incorrectly interprets this as a failure due to a >> lack of memory. >> >> ~~This PR modifies ClassLoader.c to produce an OutOfMemoryError only when >> `errno == ENOMEM` and to produce a ClassFormatError with the message >> "ClassLoader internal allocation failure" in all other cases (in which >> malloc returns NULL).~~ [edit: The above no longer describes the PR's >> proposed fix. See discussion below] >> >> In addition, I performed some minor tidy-up work in ClassLoader.c by >> changing instances of `return 0` to `return NULL`, and `if (some_ptr == 0)` >> to `if (some_ptr == NULL)`. This was done to improve the clarity of the code >> in ClassLoader.c, but didn't feel worthy of opening a separate issue. >> >> ### Alternatives >> >> It would be possible to address this failure by modifying the test to accept >> the OutOfMemoryError on AIX. I thought it was a better solution to modify >> ClassLoader.c to produce an OutOfMemoryError only when the system is >> actually out of memory. >> >> ### Testing >> >> This change has been tested on AIX and Linux/x86. > > Tyler Steele has updated the pull request incrementally with four additional > commits since the last revision: > > - Fixes type warning. > - Removes unneeded ClassFormatError from ClassLoader.c > - Revert "Extract memory error logic to helper procedure" > > This reverts commit b631eb0ccd5f3748c2010c864f8ccef0c1da9c42. > - Avoid calling malloc with size zero. Changes requested by rriggs (Reviewer). src/java.base/share/native/libjava/ClassLoader.c line 106: > 104: // NULL or a unique non-NULL pointer. To unify libc behavior across > our platforms > 105: // we chose the latter. (see 8283225) > 106: body = (jbyte *)malloc(length < 1 ? 1 : length); This code conflates a length == in the comment with length < 1 in the code. If the issue is with length == 0, make that be the test. src/java.base/share/native/libjava/ClassLoader.c line 250: > 248: // NULL or a unique non-NULL pointer. To unify libc behavior across > our platforms > 249: // we chose the latter. (see 8283225) > 250: body = (jbyte *)malloc(length < 1 ? 1 : length); The comment talks about a length of zero, but the code does something else with length < 1. I'd rather not see negative sizes conflated with zero. test/hotspot/jtreg/runtime/DefineClass/NullClassBytesTest.java line 26: > 24: /* > 25: * @test > 26: * @bug 8262913 8283225 All of the changes can be removed from NullClassBytesTest.java. (copyright and bug #) ------------- PR: https://git.openjdk.java.net/jdk/pull/7829
