Request for Comments: Potential leak of memory pointed to by 'name' about jvmciCodeInstaller

[Leslie Zhai]

Hi,

Bug reported[1] by the clang static analyzer.

Description: Potential leak of memory pointed to by 'name'
File: /home/zhaixiang/jdk/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp
Line: 653

652  char* name = strdup(java_lang_String::as_utf8_string(stubName));

      5 ← Memory is allocated  →

653  cb = RuntimeStub::new_runtime_stub(name,

      6 ← Potential leak of memory pointed to by 'name'

I checked `install` function in 
src/hotspot/share/jvmci/jvmciCodeInstaller.cpp and `installCode` 
C2V_VMENTRY in src/hotspot/share/jvmci/jvmciCompilerToVM.cpp carefully.  
There is no `free` to release the allocated memory, so I argue that it 
is a Memory leak issue, not a False positive[2]. May I file a bug if it 
is real potential leak of memory issue?

Because I think webrev is related to BUGID[3], so I just paste my patch 
here:


----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---
diff -r 1a18b8d56d73 src/hotspot/share/jvmci/jvmciCodeInstaller.cpp
--- a/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp    Sat Mar 16 15:05:21 2019 -0700
+++ b/src/hotspot/share/jvmci/jvmciCodeInstaller.cpp    Sun Mar 17 17:06:50 2019 +0800
@@ -623,7 +623,7 @@
  #endif // INCLUDE_AOT
  
  // constructor used to create a method
-JVMCIEnv::CodeInstallResult CodeInstaller::install(JVMCICompiler* compiler, Handle target, Handle compiled_code, CodeBlob*& cb, Handle installed_code, Handle speculation_log, TRAPS) {
+JVMCIEnv::CodeInstallResult CodeInstaller::install(JVMCICompiler* compiler, Handle target, Handle compiled_code, CodeBlob*& cb, char*& cb_name, Handle installed_code, Handle speculation_log, TRAPS) {
    CodeBuffer buffer("JVMCI Compiler CodeBuffer");
    jobject compiled_code_obj = JNIHandles::make_local(compiled_code());
    OopRecorder* recorder = new OopRecorder(&_arena, true);
@@ -649,8 +649,8 @@
      if (stubName == NULL) {
        JVMCI_ERROR_OK("stub should have a name");
      }
-    char* name = strdup(java_lang_String::as_utf8_string(stubName));
-    cb = RuntimeStub::new_runtime_stub(name,
+    cb_name = strdup(java_lang_String::as_utf8_string(stubName));
+    cb = RuntimeStub::new_runtime_stub(cb_name,
                                         &buffer,
                                         CodeOffsets::frame_never_safe,
                                         stack_slots,
diff -r 1a18b8d56d73 src/hotspot/share/jvmci/jvmciCodeInstaller.hpp
--- a/src/hotspot/share/jvmci/jvmciCodeInstaller.hpp    Sat Mar 16 15:05:21 2019 -0700
+++ b/src/hotspot/share/jvmci/jvmciCodeInstaller.hpp    Sun Mar 17 17:06:50 2019 +0800
@@ -207,7 +207,7 @@
  #if INCLUDE_AOT
    JVMCIEnv::CodeInstallResult gather_metadata(Handle target, Handle compiled_code, CodeMetadata& metadata, TRAPS);
  #endif
-  JVMCIEnv::CodeInstallResult install(JVMCICompiler* compiler, Handle target, Handle compiled_code, CodeBlob*& cb, Handle installed_code, Handle speculation_log, TRAPS);
+  JVMCIEnv::CodeInstallResult install(JVMCICompiler* compiler, Handle target, Handle compiled_code, CodeBlob*& cb, char*& cb_name, Handle installed_code, Handle speculation_log, TRAPS);
  
    static address runtime_call_target_address(oop runtime_call);
    static VMReg get_hotspot_reg(jint jvmciRegisterNumber, TRAPS);
diff -r 1a18b8d56d73 src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
--- a/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp     Sat Mar 16 15:05:21 2019 -0700
+++ b/src/hotspot/share/jvmci/jvmciCompilerToVM.cpp     Sun Mar 17 17:06:50 2019 +0800
@@ -677,6 +677,7 @@
    Handle target_handle(THREAD, JNIHandles::resolve(target));
    Handle compiled_code_handle(THREAD, JNIHandles::resolve(compiled_code));
    CodeBlob* cb = NULL;
+  char* cb_name = NULL;
    Handle installed_code_handle(THREAD, JNIHandles::resolve(installed_code));
    Handle speculation_log_handle(THREAD, JNIHandles::resolve(speculation_log));
  
@@ -685,7 +686,7 @@
    TraceTime install_time("installCode", JVMCICompiler::codeInstallTimer());
    bool is_immutable_PIC = HotSpotCompiledCode::isImmutablePIC(compiled_code_handle) > 0;
    CodeInstaller installer(is_immutable_PIC);
-  JVMCIEnv::CodeInstallResult result = installer.install(compiler, target_handle, compiled_code_handle, cb, installed_code_handle, speculation_log_handle, CHECK_0);
+  JVMCIEnv::CodeInstallResult result = installer.install(compiler, target_handle, compiled_code_handle, cb, cb_name, installed_code_handle, speculation_log_handle, CHECK_0);
  
    if (PrintCodeCacheOnCompilation) {
      stringStream s;
@@ -722,6 +723,7 @@
        }
      }
    }
+  if (cb_name) free(cb_name);
    return result;
  C2V_END
  

----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---


I ran clang static analyzer again, and it is not reproducible owing to I 
fixed the issue, not False negative :)

hotspot:tier1 linux-x86_64-server-fastdebug 2 fail:

* compiler/c2/Test8062950.java: it is also reproducible for mips64el 
without the patch
* runtime/classFileParserBug/TestEmptyBootstrapMethodsAttr.java: Test 
empty bootstrap_methods table within BootstrapMethods attribute


Please point out my any fault!

Thanks,

Leslie Zhai

[1] 
https://raw.githubusercontent.com/xiangzhai/jdk-dev/master/jvmciCodeInstaller.cpp.png

[2] https://bugs.llvm.org/show_bug.cgi?id=40913

[3] 
https://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-September/007855.html