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

[Doug Simon]

Hi Leslie,

As a point of process, I think hotspot-compiler-dev at openjdk.java.net is probably a better list for JVMCI bugs.

In any case, thanks for the investigation. However, I don’t think this is a bug as RuntimeStub simply passes along the name argument which is eventually stored to CodeBlob::_name without any further copying. If we subsequently freed that argument, the CodeBlob::_name would become invalid.

-Doug

> On 17 Mar 2019, at 10:30, Leslie Zhai <zhaixiang at loongson.cn> wrote:
> 
> 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
> 
>