8325095: C2: bailout message broken: ResourceArea allocated string used after free #17710

eme64 · 2024-02-05T16:02:57Z

Problem
Bailout failure_reason strings used to be either nullptr or a static string. But with JDK-8303951 I had introduced dynamic strings (e.g. formatted using stringStream::as_string()). These dynamic strings were ResourceArea allocated, and have a very limited life-span, i.e. withing the most recent ResourceMark scope. The pointer is passed to Compile::_failure_reason, which already might leave that ResourceMark scope, and the pointer is invalid. And then the pointer is further passed to ciEnv::_failure_reason. And finally, it even gets passed up to CompileBroker::invoke_compiler_on_method. When the string is finally printed for the bailout message, we already left original ResourceMark scope, and also the Compile and ciEnv scopes. The pointer now points to basically random data.

Solution
Whenever a string is passed to an outer scope, I make a CHeap copy, and the outer scope is the owner of that copy.
This way every scope is the owner of its own copy, and can allocate and deallocate according to its own strategy safely.

I introduced a utility class CHeapStringHolder:

set: make local copy on CHeap.
clear: free local copy. We before set, and in the destructor. Thus, when the holder goes out of scope, the memory is automatically freed.

We have these 4 scopes:

ResourceMark: It allocates from ResourcArea, and deallocation is taken care at the end of the ResourceMark scope.
Compile: We turn the _failure_reason from a char* into a CHeapStringHolder. We set _failure_reason while the ResourceMark is live.
ciEnv: We turn the _failure_reason from a char* into a CHeapStringHolder. We set _failure_reason while Compile is live.
CompileTask: We used to just set failure_reason, which assumes that the string is static or elsewhere managed. Now I use pattern available in other places of CompileBroker::invoke_compiler_on_method: duplicate the string with os::strdup from some other scope, and set reason_on_C_heap = true, which means that we are the owner of that copy, and are responsible for freeing it later. Not sure if that is a nice pattern, but I don't want to refactor that code and it does what I need it to do.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8325095: C2: bailout message broken: ResourceArea allocated string used after free (Bug - P4)

Reviewers

Vladimir Kozlov (@vnkozlov - Reviewer) ⚠️ Review applies to 894a642c
Dean Long (@dean-long - Reviewer) ⚠️ Review applies to 894a642c

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/17710/head:pull/17710
$ git checkout pull/17710

Update a local copy of the PR:
$ git checkout pull/17710
$ git pull https://git.openjdk.org/jdk.git pull/17710/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17710

View PR using the GUI difftool:
$ git pr show -t 17710

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/17710.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-02-05T16:03:44Z

👋 Welcome back epeter! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-02-05T16:04:28Z

@eme64 The following label will be automatically applied to this pull request:

hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

jdksjolen · 2024-02-06T10:18:26Z

Hi,

I don't agree with the way this patch works and I think we're bound for trouble. Why not store an instance of this class inside of the task instead? It takes a bit more space (136 bytes) but it actually handles its own lifetime.

class CompilationFailure {
  stringStream failure_reason;
  CompilationFailure() : failure_reason() {}
  void replace_failure(const char* reason) { 
    failure_reason.reset();
    failure_reason.print_raw(reason);
  }
  const stringStream& reason() const {
    return failure_reason;
  }
}

eme64 · 2024-02-06T10:30:32Z

@jdksjolen

I don't agree with the way this patch works and I think we're bound for trouble.

Why?

Essencially, your solution would just be a better version of what we have with failure_reason_on_C_heap.

Your suggestion would require that there is only a single failure_reason around at any given time. But I'm not sure if there is really only a single reason around, at least they are set at different places, and can be replaced... That is why I was looking for something that can keep the lifetime of multiple strings.

I'm not saying I'm proud of the draft-solution yet 🤣

These strings are stored at the level of the compilation, ciEnv (outlives compilation), and CompileTask (outlives ciEnv). So I need to allocate the string from somewhere that persists when I format the string, and then pass it into record_method_not_compilable. Currently, there is no clear ownership of the strings, and we did not need that as long as all the strings were static - but that has changed.

Maybe I can refactor it all and make sure there is only a single reason at any time. Not sure if that is feasible or how much work that would be.

One issue is that failure_reasons, and especially dynamic failure_reasons are rare, and not really tested. So it is hard to see if I would break things.

jdksjolen · 2024-02-06T11:13:22Z

I don't like it because you're leaking memory over the lifetime of the CompileTask and because there are simpler solutions, as we only ever store one failure reason.

Your suggestion would require that there is only a single failure_reason around at any given time. But I'm not sure if there is really only a single reason around, at least they are set at different places, and can be replaced... That is why I was looking for something that can keep the lifetime of multiple strings.

How would multiple strings be accessed through one const pointer :-)?

Essencially, your solution would just be a better version of what we have with failure_reason_on_C_heap.

Yes, the lifetime is managed correctly and automatically by the RAII object. Plus, the bool can be removed. One issue is that you will probably have to add a bool has_reason to the class I suggested, we can't depend on something being nullptr to indicate absence of reason anymore.

eme64 · 2024-02-06T11:24:18Z

@jdksjolen

How would multiple strings be accessed through one const pointer :-)?

Well, there are multiple pointers 🙈
grep "failure_reason" src/hotspot/share/ -r | grep "char*"

I know that the strings are passed between Compile, ciEnv and CompileTask. This means we would have multiple owners, and that is not great. Again: maybe that can be refactored, but it would take me some time and it would be risky because we don't have good testing for failure_reason reporting. Feel free to dive into the different record_method_not_compilable methods yourself.

What about having a new CHeapString class? It could hold a string (or null, or even special case for static string). We pass that around. The copy-constructor just re-allocates the string on CHeap, the destructor frees the string (unless static or null). This all comes at the cost of some more (occasional, probably rare) allocations.

jdksjolen · 2024-02-06T11:54:07Z

You don't need a new string class, you can use stringStream as that will hold the string memory and its local copy. I've looked at the code and we have 3 separate pointers to a failure reason (as you said), one in ciEnv, one in Compile, and one in CompileTask. Instead of using pointers, use the class I suggested and have these 3 have ownership of their own copies of these strings. The allocations' cost is irrelevant :-).

eme64 · 2024-02-06T12:05:18Z

@jdksjolen
Ok, so this new approach would have every place that currently has a pointer become a proper "owner" that manages the life-time of its string.

I think I prefer a new string-class, that also allows for null and possibly static strings (no allocation). Otherwise, if I use stringStream, then I would either have to initialize them as empty, and check for empty (-> no failure_reason). And I also would have to always export the actual string over as_string, which copies one more time to either ResourceArea or CHeap.

Thanks for chiming in here, Johan :)

jdksjolen · 2024-02-06T12:09:00Z

Depending on the usage you'll just have to use .freeze() or .base() which will give you the underlying string array, no copying needed!

eme64 · 2024-02-09T19:53:28Z

@jdksjolen I tried it with such a string class. And then I tried to use it for ciEnv::_failure_reason. But now I run into this:

/oracle-work/jdk-fork3/open/src/hotspot/share/runtime/vmStructs.cpp:729:3: note: in expansion of macro 'CHECK_NONSTATIC_VM_STRUCT_ENTRY'
  729 |   nonstatic_field(ciEnv,                       _failure_reason,                               const char*)                           \

Any suggestions with that?

Update: I think it is actually not used by the servicability agent, and I can just delete that reference.

eme64 · 2024-02-14T17:32:19Z

src/hotspot/share/compiler/compilationMemoryStatistic.cpp

@@ -405,7 +405,7 @@ void CompilationMemoryStatistic::on_end_compilation() {
  if (env) {
    const char* const failure_reason = env->failure_reason();
    if (failure_reason != nullptr) {
-      result = (failure_reason == failure_reason_memlimit()) ? "oom" : "err";
+      result = (strcmp(failure_reason, failure_reason_memlimit()) == 0) ? "oom" : "err";


Note: Now that ciEnv makes a local copy, we cannot just compare pointers, we need to compare the content.

eme64 · 2024-02-15T06:49:43Z

Thanks @jdksjolen for your suggestions! I now found a solution I'm fine with, and that seems to work.

mlbridge · 2024-02-15T06:54:20Z

Webrevs

dean-long · 2024-02-15T08:05:43Z

Back when I filed JDK-8132354, I proposed that both C1 and C2 delegate to ciEnv. Now there is a 3rd layer/scope of error reporting. I don't think the solution needs to be so complicated. Even with the new formatted strings, the maximum possible length can be determined (I don't see it using klass or method names). So why can't the storage simply be char[MAX_COMPILER_ERROR_LEN] in CompilerThread, and setting it be a strncpy?

eme64 · 2024-02-15T18:04:20Z

src/hotspot/share/ci/ciEnv.cpp

@@ -1265,7 +1268,7 @@ void ciEnv::record_method_not_compilable(const char* reason, bool all_tiers) {
    _compilable = new_compilable;

    // Reset failure reason; this one is more important.
-    _failure_reason = nullptr;
+    _failure_reason.clear();
    record_failure(reason);


@dean-long the tricky part is here, I think:
We are in ciEnv::record_method_not_compilable. Depending on above condition, we either update the failure reason, or don't. So there may already be an old failure reason (the first one) that we overwrite (with a second one) or leave.

This method is called from Compile::record_method_not_compilable. There, we also call Compile::record_failure, which sets Compile::_failure_reason, with no such condition.

We might for example decide to keep the old failure reason at ciEnv, but update the failure reason at Compile, at least that is how I read the code.

Hence, there can be multiple failure reasons around, I think. And I cannot fit multiple messages in a single buffer. Thus, I need some way to dynamically allocate.

Now: is this good or desirable? I don't know. Potentially having different failure messages at Compile and ciEnv - not sure if that is reasonable. We should probably discuss that. But for now I'm just trying to fix the code the way it works.

What do you think?

I think initially we care only about "most" important failure because implementation allowed to report only one failure.
If you can keep all failures AND report all of them it would be nice, I think. May be in separate RFE.

We should have only one place where we keep failures.

Reporting all failures might make sense for logging, but we can only report one reason, unless we want to do something like chained exceptions, which I don't think we need. The different layers is already looking like a custom exception handling mechanism, unfortunately.

Piling on... I opt for keeping this code simple. A list of subsequent failure states could easily be part of logging.

Okay, I am voting for UL to print all failures we hit during compilation and keep reporting only one reason at the end.

log_error(compilation) (failiure_reason);

Or log_warning()

@vnkozlov @dean-long @tstuefe
Ok, I mean logging of additional failure reasons might even be done in a separate RFE, right?
But don't we already log the failures? I mean there is the log()->elem( code.

And should I still keep the "most important" failure reason?

vnkozlov

I thought after our offline discussion that you will keep failure reasons in CompileTask.
What happened to that approach?

eme64 · 2024-02-15T23:03:44Z

@vnkozlov yes, that was the plan. But then @jdksjolen objected, see his comments above. The new approach would be a bit more "clean", as the ownership is now clear.
The problem with the arena: I can't use a simple Arena at CompileTask, since an Arena only grows. A CompileTask is reused for many compilations, and so potentially we would have many failures on that Arena, leading to uncontrolled memory usage. A ResourceMark would require a ResourceMark - that could be done I guess. But creating such ad-hoc ResourceAreas with their own ResourceMarks seems a bit ... well ad-hoc. The ResourceArea would live in CompileTask, but the ResourceMark would only show up where the CompileTaskWrapper is introduced. Just seems very "hacky".
So that is why @jdksjolen proposed to just have a string-holder that simply allocates itself. Allocation and ownership are straight forward this way.

Any yes, there should probably be some refactoring. I'm not sure why there are failure_reason reportings at 3 layers (maybe even more?): Compile, ciEnv, CompileTask. Maybe we can log a list of failure reasons only at CompileTask level? I'd have to see if that is a feasible refactoring.

Would it be ok to push this BugFix (for backport), and then come up with a refactoring RFE? We probably also want to refactor the strange reason_on_C_heap mechanism on CompileTask level. If someone sets that flag wrong, we have a bad free, or a memory leak.

dholmes-ora · 2024-02-16T06:40:06Z

src/hotspot/share/utilities/cHeapStringHolder.cpp

+    // Make sure it is written before the pointer is used again
+    OrderAccess::storestore();


Is this internal string being exposed to other threads ??? That seems dangerous in general given this is a stackobj.

@dholmes-ora I don't think it is exposed to other threads. It only goes as far up as CompileTask, so I think it always stays in the compile thread. I saw OrderAccess::storestore() in the stringStream code. But maybe I don't need it here?

OrderAccess is only needed for exchanging data across threads.

@dholmes-ora ok, thanks for the comment! I removed it now.

tstuefe · 2024-02-16T12:27:45Z

src/hotspot/share/ci/ciEnv.cpp

@@ -1265,7 +1268,7 @@ void ciEnv::record_method_not_compilable(const char* reason, bool all_tiers) {
    _compilable = new_compilable;

    // Reset failure reason; this one is more important.
-    _failure_reason = nullptr;
+    _failure_reason.clear();
    record_failure(reason);


Piling on... I opt for keeping this code simple. A list of subsequent failure states could easily be part of logging.

tstuefe · 2024-02-16T12:34:24Z

src/hotspot/share/utilities/cHeapStringHolder.hpp

+  CHeapStringHolder(MEMFLAGS memflags) :
+    _memflags(memflags), _string(nullptr) {}
+  ~CHeapStringHolder() { clear(); };
+  NONCOPYABLE(CHeapStringHolder);


I'd opt for a use-case-specific class inside compiler. It would allow you to hard-code mtCompiler, simplifying the code, and to log with UL into the compiler log when the failure reason is changed.

A new generic string class opens a whole can of bikeshedding worms, e.g. whether to make it mutable, how best to specify MEMFLAGS etc... I would avoid that, especially since the added value fo this helper class is slim.

I agree this should be in compiler/

Ok, I can do that. Just wanted it to be shared code in case someone needs it in the future. But maybe that is too hard to do, due to bikeshedding... :/

Not sure I want to UL here, since I hold the string in multiple places, that would lead to multiple loggings.

vnkozlov

Few comments.

vnkozlov · 2024-02-17T00:34:58Z

src/hotspot/share/compiler/compileBroker.cpp

-      failure_reason = ci_env.failure_reason();
+      // Duplicate the failure reason string, so that it outlives ciEnv
+      failure_reason = os::strdup(ci_env.failure_reason(), mtCompiler);
+      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 😅 :

vnkozlov · 2024-02-17T00:53:12Z

src/hotspot/share/opto/compile.hpp

@@ -363,7 +364,7 @@ class Compile : public Phase {
  ciEnv*                _env;                   // CI interface
  DirectiveSet*         _directive;             // Compiler directive
  CompileLog*           _log;                   // from CompilerThread
-  const char*           _failure_reason;        // for record_failure/failing pattern
+  CHeapStringHolder     _failure_reason;       // for record_failure/failing pattern


wrong comment's indent

vnkozlov · 2024-02-17T00:55:15Z

src/hotspot/share/utilities/cHeapStringHolder.hpp

+  CHeapStringHolder(MEMFLAGS memflags) :
+    _memflags(memflags), _string(nullptr) {}
+  ~CHeapStringHolder() { clear(); };
+  NONCOPYABLE(CHeapStringHolder);


I agree this should be in compiler/

vnkozlov · 2024-02-17T01:34:56Z

src/hotspot/share/ci/ciEnv.cpp

@@ -1265,7 +1268,7 @@ void ciEnv::record_method_not_compilable(const char* reason, bool all_tiers) {
    _compilable = new_compilable;

    // Reset failure reason; this one is more important.
-    _failure_reason = nullptr;
+    _failure_reason.clear();
    record_failure(reason);


Okay, I am voting for UL to print all failures we hit during compilation and keep reporting only one reason at the end.

log_error(compilation) (failiure_reason);

Or log_warning()

vnkozlov · 2024-02-20T16:59:03Z

UL should be done in separate RFE. Current logging (log()->elem()) is for -XX:+LogCompilation output into file. UL is for terminal.
UL should report immediately, the same way we do for -XX:+LogCompilation.

Different failures are recorded in ciEnv and Compiler, only after compilation is finish we record compiler's failure in ciEnv.
There is also ciTypefFow failure which recorded in Compiler.

eme64 · 2024-02-23T08:49:47Z

@vnkozlov @tstuefe thanks for the comments and suggestions!

What I did:

Moved utilities/cHeapStringHolder.hpp/cpp -> compiler/cHeapStringHolder.hpp/cpp.
Directly use mtCompiler for it, rather than constructor argument with MEMFLAGS.
Fixed 2 issues (bugs and code style)

What I did not yet do:
Any changes to logging. I think we want to do that in a separate RFE anyway, and not backport it together with this bugfix.
Is there anything that you still need me to change for this bugfix here?

About Logging, in a future RFE:
@vnkozlov suggested to always UL failures immediately. The question is where I should do that. At ciEnv::record_method_not_compilable only? Or also in Compile::record_method_not_compilable?
And @vnkozlov also suggested only to report one (a single) failure reason at the very end, presumably at the CompileTask level. Does that mean you would still overwrite the less important failures with more important ones in ciEnv, like we do currently?

vnkozlov · 2024-02-23T18:31:44Z

About Logging, in a future RFE:
@vnkozlov suggested to always UL failures immediately. The question is where I should do that. At ciEnv::record_method_not_compilable only? Or also in Compile::record_method_not_compilable?

Compile::record_method_not_compilable() calls ciEnv::record_method_not_compilable() so you need only add it to ciEnv.

And @vnkozlov also suggested only to report one (a single) failure reason at the very end, presumably at the CompileTask level. Does that mean you would still overwrite the less important failures with more important ones in ciEnv, like we do currently?

I suggest to leave the normal report code as it is now (one failure). And if we need additional information we will use UL when we implement it.

Most ciEnv failures are recorded (record_failure) during publishing compiled code in ciEnv::register_method() after compilation is done and it did not have failures.

vnkozlov

This version looks good to me.

openjdk · 2024-02-23T20:24:08Z

@eme64 This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8325095: C2: bailout message broken: ResourceArea allocated string used after free

Reviewed-by: kvn, dlong

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 77 new commits pushed to the master branch:

12404a5: 8324790: ifnode::fold_compares_helper cleanup
0d35450: 8327040: Problemlist ActionListenerCalledTwiceTest.java test failing in macos14
8d6f784: 8327056: Remove unused static char array in JvmtiAgentList::lookup
43af120: 8326959: Improve JVMCI option help
742c776: 8322743: C2: prevent lock region elimination in OSR compilation
d29cefb: 8326838: JFR: Native mirror events
b8fc418: 8326525: com/sun/tools/attach/BasicTests.java does not verify AgentLoadException case
d9aa1de: 8318605: Enable parallelism in vmTestbase/nsk/stress/stack tests
bbfda65: 8326897: (fs) The utility TestUtil.supportsLinks is wrongly used to check for hard link support
db0e2b8: 8326944: (ch) Minor typo in the ScatteringByteChannel.read(ByteBuffer[] dsts,int offset,int length) javadoc
... and 67 more: https://git.openjdk.org/jdk/compare/09a78b5da961f3575cf20c9b56bff86ddbd6545d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

eme64 · 2024-02-29T11:54:21Z

@dean-long @tstuefe @dholmes-ora can anybody give me a second review, please?

dholmes-ora · 2024-03-01T06:05:38Z

src/hotspot/share/compiler/cHeapStringHolder.cpp

+ */
+
+#include "precompiled.hpp"
+#include "runtime/orderAccess.hpp"


You don't need this include

Your are right! Removed it.

eme64 · 2024-03-05T06:35:13Z

Thanks @vnkozlov @dlunde @tstuefe @dholmes-ora for reviewing / your suggestions!
Thanks @jdksjolen for all the memory allocation conversations!

/integrate

openjdk · 2024-03-05T06:35:34Z

Going to push as commit c589555.
Since your change was applied there have been 111 commits pushed to the master branch:

b7540df: 8327007: javax/swing/JSpinner/8008657/bug8008657.java fails
e1b661f: 8319900: Recursive lightweight locking: riscv64 implementation
045eea1: 8327057: Parallel: Refactor ParMarkBitMap::iterate
f615ac4: 8309622: Re-examine the cache mechanism in BaseLocale
6f8d351: 8325725: Parallel: Refactor PSParallelCompact::fill_dense_prefix_end
8cfaceb: 8327125: SpinYield.report should report microseconds
59529a9: 8326688: Parallel: Remove unnecessary BOT update in UpdateOnlyClosure::do_addr
43c6f0b: 8326591: New test JmodExcludedFiles.java fails on Windows when --with-external-symbols-in-bundles=public is used
b5cd7ef: 8319901: Recursive lightweight locking: ppc64le implementation
0583f73: 8323183: ClassFile API performance improvements
... and 101 more: https://git.openjdk.org/jdk/compare/09a78b5da961f3575cf20c9b56bff86ddbd6545d...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-03-05T06:35:39Z

@eme64 Pushed as commit c589555.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8325095

c614857

openjdk bot changed the title ~~8325095~~ 8325095: C2: bailout message broken: ResourceArea allocated string used after free Feb 5, 2024

openjdk bot added the hotspot hotspot-dev@openjdk.org label Feb 5, 2024

use ResourceArea instead of Arena, to make lifetime smaller

11e6e52

add string holder class

59311e9

eme64 added 7 commits February 12, 2024 18:18

fixed ciEnv

a0753e6

remove use of resource_area

4dc2cc0

copy string from ciEnv

eb7ba45

more fix

ce6321f

Merge branch 'master' into JDK-8325095

550fc11

cleanup debug prints

e790e8e

strcmp for MemStat

c5b75b7

eme64 commented Feb 14, 2024

View reviewed changes

eme64 marked this pull request as ready for review February 15, 2024 06:49

openjdk bot added the rfr Pull request is ready for review label Feb 15, 2024

eme64 commented Feb 15, 2024

View reviewed changes

vnkozlov reviewed Feb 15, 2024

View reviewed changes

dholmes-ora reviewed Feb 16, 2024

View reviewed changes

remove OrderAccess::storestore() bc single threaded only

127e08c

tstuefe reviewed Feb 16, 2024

View reviewed changes

vnkozlov suggested changes Feb 17, 2024

View reviewed changes

eme64 added 3 commits February 23, 2024 09:39

address review suggestions

14af05c

more review suggestion responses

fd43821

restore code to previous shape

f37b66b

eme64 added 2 commits February 23, 2024 16:26

Merge branch 'master' into JDK-8325095

00bcb27

include precompiled.hpp, otherwise windows build fails

894a642

vnkozlov approved these changes Feb 23, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Feb 23, 2024

dean-long approved these changes Mar 1, 2024

View reviewed changes

dholmes-ora reviewed Mar 1, 2024

View reviewed changes

remove unnecessary include

7cf5c20

openjdk bot added the integrated Pull request has been integrated label Mar 5, 2024

openjdk bot closed this Mar 5, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 5, 2024

		// Make sure it is written before the pointer is used again
		OrderAccess::storestore();

8325095: C2: bailout message broken: ResourceArea allocated string used after free #17710

8325095: C2: bailout message broken: ResourceArea allocated string used after free #17710

Conversation

eme64 commented Feb 5, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 5, 2024

openjdk bot commented Feb 5, 2024

jdksjolen commented Feb 6, 2024

eme64 commented Feb 6, 2024 • edited Loading

jdksjolen commented Feb 6, 2024

eme64 commented Feb 6, 2024 • edited Loading

jdksjolen commented Feb 6, 2024

eme64 commented Feb 6, 2024

jdksjolen commented Feb 6, 2024

eme64 commented Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

eme64 commented Feb 15, 2024

mlbridge bot commented Feb 15, 2024 • edited Loading

Webrevs

dean-long commented Feb 15, 2024

eme64 Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnkozlov left a comment

Choose a reason for hiding this comment

eme64 commented Feb 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnkozlov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eme64 Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnkozlov commented Feb 20, 2024

eme64 commented Feb 23, 2024

vnkozlov commented Feb 23, 2024

vnkozlov left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 23, 2024 • edited Loading

eme64 commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eme64 commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

openjdk bot commented Mar 5, 2024

eme64 commented Feb 5, 2024 •

edited by openjdk bot

Loading

eme64 commented Feb 6, 2024 •

edited

Loading

eme64 commented Feb 6, 2024 •

edited

Loading

eme64 commented Feb 9, 2024 •

edited

Loading

mlbridge bot commented Feb 15, 2024 •

edited

Loading

eme64 Feb 15, 2024 •

edited

Loading

eme64 commented Feb 15, 2024 •

edited

Loading

eme64 Feb 19, 2024 •

edited

Loading

openjdk bot commented Feb 23, 2024 •

edited

Loading