RFR: 8283225: [AIX] ClassLoader.c produces incorrect OutOfMemory Exception when length is 0
Thomas Stuefe
stuefe at openjdk.java.net
Wed Mar 16 14:41:41 UTC 2022
On Tue, 15 Mar 2022 22:58:48 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).
>
> 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.
Hi Tyler,
The way we solve this usually is by homogenizing malloc behavior across all platforms with `if (len == 0) len=1;`, see e.g. https://github.com/openjdk/jdk/blob/08cadb4754da0d5e68ee2df45f4098d203d14115/src/hotspot/share/runtime/os.cpp#L644-L647
I suggest you do that here too since it would be a far less intrusive change. If you don't want to fix up all three malloc call sites, just reroute them to a local malloc stub (`my_malloc()`) that corrects length and calls the real malloc. If you want, you can `#ifdef _AIX` that length correction too, e.g.:
static void* my_malloc(size_t l) {
#ifdef _AIX
if (l == 0) l = 1;
#endif
return malloc(l);
}
That would be a far smaller change, and it would not introduce a new behavioral difference (because with your change, AIX now throws ClassFormatError where other platforms don't).
Side note: nothing against changing 0 to NULL, but please in a separate cleanup patch.
Cheers, Thomas
Btw, which malloc call was the problematic exactly? Cannot be the one in getUTF, since that one already adds len + 1 and never gets called with a zero length anyway.
-------------
Changes requested by stuefe (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7829
More information about the core-libs-dev
mailing list