RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0 [v3]

[David Holmes]

On Wed, 16 Mar 2022 21:10:14 GMT, Tyler Steele <duke at 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   Addresses failure in NullClassTest on AIX.
>   
>   - Changes malloc(0) call to malloc(1) on AIX.

src/java.base/share/native/libjava/ClassLoader.c line 102:

> 100:     }
> 101: 
> 102:     // On malloc(0), implementators of malloc(3) have the choice to return either

It is confusing to mix `malloc(0)`, where you are passing an argument zero to malloc, with `malloc(3)` which actually means the definition of malloc as per the man page in section 3.

Given this is only an issue on AIX the comment can simply say:

`// On AIX malloc(0) returns NULL which looks like an out-of-memory condition; so adjust it to malloc(1)`.

src/java.base/share/native/libjava/ClassLoader.c line 106:

> 104:     // we chose the latter. (see 8283225)
> 105:     #ifdef _AIX
> 106:     body = (jbyte *)malloc(length == 0 ? 1 : length);

Using AIX_ONLY this can be simplified:

`body = (jbyte *)malloc(length AIX_ONLY( == 0 ? 1 : length));`

-------------

PR: https://git.openjdk.java.net/jdk/pull/7829