[cfe-dev] Request for Comments: Potential leak of memory pointed to by 'name' about jvmciCodeInstaller

[Doug Simon via cfe-dev]

> On 18 Mar 2019, at 03:55, Leslie Zhai <zhaixiang at loongson.cn> wrote:
> 
> 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 <mailto: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` :)

Indeed. RuntimeStubs are never freed as they are used as helpers for “normal” compiled code. Once created, their addresses are stored in static variables that live for the remainder of the VM process.

-Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190318/a3f41afb/attachment.html>