RFR: 8354119: Missing C2 proper allocation failure handling during initialization (during generate_uncommon_trap_blob)

[Damon Fenacci]

On Tue, 22 Apr 2025 16:27:25 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> After [JDK-8347406](https://bugs.openjdk.org/browse/JDK-8347406), `OptoRuntime::generate_uncommon_trap_blob` and `OptoRuntime::generate_exception_blob` return an `UncommonTrapBlob`/`ExceptionBlob` if they succeed, `nullptr` if they don't. This is then used by the compiler to shut down gently if the code cache is full (instead of crashing).
>> Unfortunately if the the full code cache is reached when creating the buffer at the start of these 2 methods (when calling `CodeBuffer buffer(name, 2048, 1024);`) an empty buffer is created, which in turn prevents `masm` to be properly initialized, which then causes an access violation when writing into the blob's address when first adding `subptr` later in the method (as seen in the snippet below for `generate_uncommon_trap_blob`).
>> 
>> https://github.com/openjdk/jdk/blob/3cc43b3224efdf1a3f35fff58b993027a9e1f4ad/src/hotspot/cpu/x86/runtime_x86_64.cpp#L55-L72
>> 
>> To fix this I suggest we return immediately from `OptoRuntime::generate_uncommon_trap_blob`/`OptoRuntime::generate_exception_blob` if the `buffer` creation failed.
>> 
>> ### Testing
>> 
>> Tier 1-3.
>> No specific regression test is added (very hard, i.a. dependent on thread scheduling. On the other hand `StartupOutput.java` might catch it rarely).
>
> I finally found why `Assembler()` did not throw error when code blob is not allocated and `_blob` is `NULL`: [assembler.cpp#L47](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/asm/assembler.cpp#L47)
> 
> In **debug** VM `CodeBuffer::set_blob()` replaces `NULLs` with `basAddress` value: [codeBuffer.cpp#L184](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/asm/codeBuffer.cpp#L184). It is done to "poison" pointers when `free_blob()` is called.
> 
> So we could fix the issue by adding check for `badAddress` in `Assembler()`. But it will cause VM exit instead of disabling only C2 implemented by #23630.  On other hand it will match behavior of **product** VM.
> 
> I think I prefer current suggested fix to disable only C2.

Thanks @vnkozlov and @chhagedorn for your reviews!

As I touched a few platform specific files I wouldn't mind having it tested on each one (even just low tiers, just to make sure I didn't involuntarily screw up something): @offamitkumar (S390), @TheRealMDoerr (PPC), @RealFYang (RISC-V), @bulasevich (ARM32) would you mind? Thanks a lot!

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

PR Comment: https://git.openjdk.org/jdk/pull/24549#issuecomment-2823610502