[cfe-dev] Request for Comments: Potential leak of memory pointed to by 'name' about jvmciCodeInstaller
Leslie Zhai via cfe-dev
cfe-dev at lists.llvm.org
Sun Mar 17 19:55:58 PDT 2019
Hi Doug,
Thanks for your kind response!
在 2019年03月18日 05:55, 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.
Sorry for my wrong posting!
>
> 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.
Thanks for pointing out my fault!
I might brought in Use-after-free[2] issue even that I carefully free
the allocated memory in the end of `installCode` C2V_VMENTRY...
The reduced testcase is able to reproduce my fault:
----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---
$ cat t.cpp
#include <iostream>
#include <string.h>
#include <stdlib.h>
class CodeBlob {
public:
CodeBlob(const char* name) : _name(name) {};
const char* name() { return _name; }
protected:
const char* _name;
};
class RuntimeBlob : public CodeBlob {
public:
RuntimeBlob(const char* name) : CodeBlob(name) {}
};
class RuntimeStub : public RuntimeBlob {
private:
RuntimeStub(const char* name) : RuntimeBlob(name) {}
public:
static RuntimeStub* new_runtime_stub(const char* stub_name) {
RuntimeStub* stub = new RuntimeStub(stub_name);
return stub;
}
};
static void install(CodeBlob*& cb, char*& name) {
name = strdup("some stubName");
cb = RuntimeStub::new_runtime_stub(name);
}
// To simulate C2V_VMENTRY(jint, installCode, (JNIEnv *jniEnv, jobject,
jobject target, jobject compiled_code, jobject installed_code, jobject
speculation_log))
int main(int argc, char *argv[]) {
CodeBlob* cb = NULL;
char* cb_name = NULL;
install(cb, cb_name);
if (cb_name) free(cb_name); // <--- MY FAULT
std::cout << cb->name() << std::endl;
return 0;
}
----- 8< -------- 8< -------- 8< -------- 8< -------- 8< -------- 8< ---
t.cpp:9:24: warning: Use of memory after it is freed
const char* name() { return _name; }
^~~~~~~~~~~~
t.cpp:42:3: warning: Potential leak of memory pointed to by 'cb'
std::cout << cb->name() << std::endl;
^~~~~~~~~~~~~~~~~~~~~~~
So It might be Potential leak of memory pointed to by 'cb' about
jvmciCompilerToVM? But the `cb` might be used in other place just like
`name` :)
Just comment MY FAULT //if (cb_name) free(cb_name);
And see what dynamic analysis say:
$ clang++ -fsanitize=address t.cpp
$ ./a.out
=================================================================
==4482==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 8 byte(s) in 1 object(s) allocated from:
#0 0x12017b674 (/home/loongson/zhaixiang/a.out+0x12017b674)
#1 0x12018097c (/home/loongson/zhaixiang/a.out+0x12018097c)
#2 0x12018074c (/home/loongson/zhaixiang/a.out+0x12018074c)
#3 0x1201804c4 (/home/loongson/zhaixiang/a.out+0x1201804c4)
#4 0xfff6cc719c (/lib64/libc.so.6+0x4319c)
#5 0x12002b198 (/home/loongson/zhaixiang/a.out+0x12002b198)
Indirect leak of 14 byte(s) in 1 object(s) allocated from:
#0 0x1200de204 (/home/loongson/zhaixiang/a.out+0x1200de204)
#1 0x120180688 (/home/loongson/zhaixiang/a.out+0x120180688)
#2 0x1201804c4 (/home/loongson/zhaixiang/a.out+0x1201804c4)
#3 0xfff6cc719c (/lib64/libc.so.6+0x4319c)
#4 0x12002b198 (/home/loongson/zhaixiang/a.out+0x12002b198)
SUMMARY: AddressSanitizer: 22 byte(s) leaked in 2 allocation(s).
Thanks,
Leslie Zhai
>
> -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
Potential Use-after-free issue reported by clang static analyzer.
>>
>> [3] https://mail.openjdk.java.net/pipermail/jdk8u-dev/2018-September/007855.html
>>
>>
More information about the cfe-dev
mailing list