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

[Thomas Stuefe]

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.

p.s. this issue puts the finger on a sore point. We have corrected mallocs in the JDK in a number of places, e.g. in libverify:

(https://github.com/openjdk/jdk/blob/3da5204b3c3a3f95bddcdcfe2628c2e0ed8a12d9/src/java.base/share/native/libverify/check_code.c#L91-L102)

(which is also strictly speaking incorrect since the spec says to return a *non-unique* pointer).

More generally, we have with AIX the problem that APIs have different behavior than the more usual nixes or are missing, and we need a porting layer to provide missing or change different APIs. Beside AIX-ish malloc, one example is dladdr, which is missing.

In hotspot, we have os/aix, so we are fine. See `os/aix/hotspot/os/aix/porting_aix.cpp`. In the JDK I never found a good place for a porting layer, since the different JDK binaries don't have a common layer. So we have multiple versions of aix malloc and dladdr sprinkled across the libraries, which I always found embarrassing. (outside hotspot we implement dladdr at least in java.desktop/aix/native/libawt/porting_aix.c and java.base/aix/native/libjli/java_md_aix.c).

If you find a way to commonize that code across JDK libraries, that would be cool. I even thought about providing a porting library completely independent from the OpenJDK itself, more like a system library. We did this for our internal iSeries port but the logistics were annoying, so we did not do it for AIX. But you at IBM may have a better idea.

Cheers, Thomas

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

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