RFR: 8325095: C2: bailout message broken: ResourceArea allocated string used after free [v2]

[Emanuel Peter]

On Sat, 17 Feb 2024 00:34:58 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   remove OrderAccess::storestore() bc single threaded only
>
> src/hotspot/share/compiler/compileBroker.cpp line 2333:
> 
>> 2331:       // Duplicate the failure reason string, so that it outlives ciEnv
>> 2332:       failure_reason = os::strdup(ci_env.failure_reason(), mtCompiler);
>> 2333:       bool reason_on_C_heap = true;
> 
> Did you mean: `failure_reason_on_C_heap = true;`?

You are right. Fixing it.

Ha, copied this from above lines:

        failure_reason = os::strdup(err_msg("Error attaching to libjvmci (err: %d, %s)",
                                    env.init_error(), msg == nullptr ? "unknown" : msg), mtJVMCI);
        bool reason_on_C_heap = true;
        // In case of JNI_ENOMEM, there's a good chance a subsequent attempt to create libjvmci or attach to it
        // might succeed. Other errors most likely indicate a non-recoverable error in the JVMCI runtime.
        bool retryable = env.init_error() == JNI_ENOMEM;
        compile_state.set_failure(retryable, failure_reason, reason_on_C_heap);

I guess here the variable is local, and `compile_state.set_failure(` "eats" up the reference (i.e. becomes the owner). But the reference to the string is also held by `failure_reason`. This is so nasty 😅 :

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17710#discussion_r1494250826