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

Reviewed-by: kvn, dlong

src/hotspot/share/ci/ciEnv.cpp

@@ -121,7 +121,6 @@ ciEnv::ciEnv(CompileTask* task)
   _oop_recorder = nullptr;
   _debug_info = nullptr;
   _dependencies = nullptr;
-  _failure_reason = nullptr;
   _inc_decompile_count_on_failure = true;
   _compilable = MethodCompilable;
   _break_at_compile = false;
@@ -250,7 +249,6 @@ ciEnv::ciEnv(Arena* arena) : _ciEnv_arena(mtCompiler) {
   _oop_recorder = nullptr;
   _debug_info = nullptr;
   _dependencies = nullptr;
-  _failure_reason = nullptr;
   _inc_decompile_count_on_failure = true;
   _compilable = MethodCompilable_never;
   _break_at_compile = false;
@@ -1233,9 +1231,9 @@ int ciEnv::num_inlined_bytecodes() const {
 // ------------------------------------------------------------------
 // ciEnv::record_failure()
 void ciEnv::record_failure(const char* reason) {
-  if (_failure_reason == nullptr) {
+  if (_failure_reason.get() == nullptr) {
     // Record the first failure reason.
-    _failure_reason = reason;
+    _failure_reason.set(reason);
   }
 }
 
@@ -1265,7 +1263,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);
   }
 }

src/hotspot/share/ci/ciEnv.hpp

@@ -33,6 +33,7 @@
 #include "code/exceptionHandlerTable.hpp"
 #include "compiler/compiler_globals.hpp"
 #include "compiler/compilerThread.hpp"
+#include "compiler/cHeapStringHolder.hpp"
 #include "oops/methodData.hpp"
 #include "runtime/javaThread.hpp"
 
@@ -57,7 +58,7 @@ private:
   OopRecorder*     _oop_recorder;
   DebugInformationRecorder* _debug_info;
   Dependencies*    _dependencies;
-  const char*      _failure_reason;
+  CHeapStringHolder _failure_reason;
   bool             _inc_decompile_count_on_failure;
   int              _compilable;
   bool             _break_at_compile;
@@ -319,10 +320,10 @@ public:
 
   // This is true if the compilation is not going to produce code.
   // (It is reasonable to retry failed compilations.)
-  bool failing() const { return _failure_reason != nullptr; }
+  bool failing() const { return _failure_reason.get() != nullptr; }
 
   // Reason this compilation is failing, such as "too many basic blocks".
-  const char* failure_reason() const { return _failure_reason; }
+  const char* failure_reason() const { return _failure_reason.get(); }
 
   // Return state of appropriate compatibility
   int compilable() { return _compilable; }

src/hotspot/share/compiler/cHeapStringHolder.cpp

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "compiler/cHeapStringHolder.hpp"
+
+void CHeapStringHolder::set(const char* string) {
+  clear();
+  if (string != nullptr) {
+    size_t len = strlen(string);
+    _string = NEW_C_HEAP_ARRAY(char, len + 1, mtCompiler);
+    ::memcpy(_string, string, len);
+    _string[len] = 0; // terminating null
+  }
+}
+
+void CHeapStringHolder::clear() {
+  if (_string != nullptr) {
+    FREE_C_HEAP_ARRAY(char, _string);
+    _string = nullptr;
+  }
+}

src/hotspot/share/compiler/cHeapStringHolder.hpp

@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_COMPILER_CHEAPSTRINGHOLDER_HPP
+#define SHARE_COMPILER_CHEAPSTRINGHOLDER_HPP
+
+#include "memory/allocation.hpp"
+
+// Holder for a C-Heap allocated String
+// The user must ensure that the destructor is called, or at least clear.
+class CHeapStringHolder : public StackObj {
+private:
+  char* _string;
+
+public:
+  CHeapStringHolder() : _string(nullptr) {}
+  ~CHeapStringHolder() { clear(); };
+  NONCOPYABLE(CHeapStringHolder);
+
+  // Allocate memory to hold a copy of string
+  void set(const char* string);
+
+  // Release allocated memory
+  void clear();
+
+  const char* get() const { return _string; };
+};
+
+#endif // SHARE_COMPILER_CHEAPSTRINGHOLDER_HPP

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";
     }
   }
 

src/hotspot/share/compiler/compileBroker.cpp

@@ -2328,7 +2328,9 @@ void CompileBroker::invoke_compiler_on_method(CompileTask* task) {
     compilable = ci_env.compilable();
 
     if (ci_env.failing()) {
-      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);
+      failure_reason_on_C_heap = true;
       retry_message = ci_env.retry_message();
       ci_env.report_failure(failure_reason);
     }

src/hotspot/share/opto/compile.cpp

@@ -639,7 +639,6 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci,
                   _env(ci_env),
                   _directive(directive),
                   _log(ci_env->log()),
-                  _failure_reason(nullptr),
                   _first_failure_details(nullptr),
                   _intrinsics        (comp_arena(), 0, 0, nullptr),
                   _macro_nodes       (comp_arena(), 8, 0, nullptr),
@@ -930,7 +929,6 @@ Compile::Compile( ciEnv* ci_env,
     _env(ci_env),
     _directive(directive),
     _log(ci_env->log()),
-    _failure_reason(nullptr),
     _first_failure_details(nullptr),
     _congraph(nullptr),
     NOT_PRODUCT(_igv_printer(nullptr) COMMA)
@@ -4392,9 +4390,9 @@ void Compile::record_failure(const char* reason) {
   if (log() != nullptr) {
     log()->elem("failure reason='%s' phase='compile'", reason);
   }
-  if (_failure_reason == nullptr) {
+  if (_failure_reason.get() == nullptr) {
     // Record the first failure reason.
-    _failure_reason = reason;
+    _failure_reason.set(reason);
     if (CaptureBailoutInformation) {
       _first_failure_details = new CompilationFailureInfo(reason);
     }

src/hotspot/share/opto/compile.hpp

@@ -32,6 +32,7 @@
 #include "compiler/compilerOracle.hpp"
 #include "compiler/compileBroker.hpp"
 #include "compiler/compilerEvent.hpp"
+#include "compiler/cHeapStringHolder.hpp"
 #include "libadt/dict.hpp"
 #include "libadt/vectset.hpp"
 #include "memory/resourceArea.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
   CompilationFailureInfo* _first_failure_details; // Details for the first failure happening during compilation
   GrowableArray<CallGenerator*> _intrinsics;    // List of intrinsics.
   GrowableArray<Node*>  _macro_nodes;           // List of nodes which need to be expanded before matching.
@@ -811,12 +812,24 @@ private:
   Arena*      comp_arena()           { return &_comp_arena; }
   ciEnv*      env() const            { return _env; }
   CompileLog* log() const            { return _log; }
-  bool        failing() const        { return _env->failing() || _failure_reason != nullptr; }
+
-  const char* failure_reason() const { return (_env->failing()) ? _env->failure_reason() : _failure_reason; }
+  bool        failing() const        {
+    return _env->failing() ||
+           _failure_reason.get() != nullptr;
+  }
+
+  const char* failure_reason() const {
+    return _env->failing() ? _env->failure_reason()
+                           : _failure_reason.get();
+  }
+
   const CompilationFailureInfo* first_failure_details() const { return _first_failure_details; }
 
   bool failure_reason_is(const char* r) const {
-    return (r == _failure_reason) || (r != nullptr && _failure_reason != nullptr && strcmp(r, _failure_reason) == 0);
+    return (r == _failure_reason.get()) ||
+           (r != nullptr &&
+            _failure_reason.get() != nullptr &&
+            strcmp(r, _failure_reason.get()) == 0);
   }
 
   void record_failure(const char* reason);

src/hotspot/share/runtime/vmStructs.cpp

@@ -724,7 +724,6 @@
   /************/                                                                                                                     \
                                                                                                                                      \
   nonstatic_field(ciEnv,                       _compiler_data,                                void*)                                 \
-  nonstatic_field(ciEnv,                       _failure_reason,                               const char*)                           \
   nonstatic_field(ciEnv,                       _factory,                                      ciObjectFactory*)                      \
   nonstatic_field(ciEnv,                       _dependencies,                                 Dependencies*)                         \
   nonstatic_field(ciEnv,                       _task,                                         CompileTask*)                          \