facebook / hhvm

A virtual machine for executing programs written in Hack.
https://hhvm.com
Other
18.12k stars 2.98k forks source link

string_bin2hex function is where the result of release(create_function) #879

Closed huzhiguang closed 10 years ago

huzhiguang commented 11 years ago

util/zend/zend_string.cpp: char string_bin2hex(const char input, int &len) { static char hexconvtab[] = "0123456789abcdef";

ASSERT(input); if (len == 0) { return NULL; }

int i, j; // this variable don't free char result = (char )malloc((len << 1) + 1);

for (i = j = 0; i < len; i++) { result[j++] = hexconvtab[(unsigned char)input[i] >> 4]; result[j++] = hexconvtab[(unsigned char)input[i] & 15]; } result[j] = '\0'; len = j; return result; }

=>HPHP::f_create_function(HPHP::String const&, HPHP::String const&) (ext_function.cpp:300) =>HPHP::VMExecutionContext::createFunction(HPHP::String const&, HPHP::String const&) (bytecode.cpp:2805) => HPHP::VM::compilestring(char const, unsigned long) (runtime.cpp:714) => HPHP::stringmd5(char const, int, bool, int&) (zend_md5.cpp:325) =>HPHP::string_bin2hex(char const*, int&) (zend_string.cpp:210)

so result is leak hot point, please author help me solve this problem

scannell commented 11 years ago

Thanks for reporting this.

huzhiguang commented 11 years ago

I can try edit compile_string function (runtime.cpp:711) free string_bin2hex funtion's result address,and should be ok,I testing this problem now,but I find other leak point in create_function . I edit compile_string code(runtime.cpp:711): //////////////////////////////////////////////////////////////////////////////////////////////////// Unit* compile_string(const char* s, size_t sz) { MD5 md5; int out_len; // md5 = MD5(string_md5(s, sz, false, out_len)); //huzhiguang edit char * string_md5_char=string_md5(s, sz, false, out_len); md5 = MD5(string_md5_char); free(string_md5_char); VM::Unit* u = Repo::get().loadUnit("", md5); if (u != NULL) { return u; } return g_hphp_compiler_parse(s, sz, md5, NULL); }

//////////////////////////////////////////////////////////////////////////////////////////////////// code position:bytecode.cpp:2792(create_function): //this code is leak point invokeFunc(&retval, unit->getMain(), Array::Create());

I find invokeFunc function in create_function memery leak and I am tracking this issue,there is progress will tell you. You can also tracking this issue with me,thanks.

huzhiguang commented 11 years ago

VMExecutionContext::createFunction call invokeFunc memery leak:::

sorry ,I provide string_bin2hex function in the create_function leak is not accurate(Issue #879 ).

I found is invokeFunc being memery leaked through long-term test,but I will continue to track this (string_bin2hex) and make sure the function is no problem.

InvokeFunc function I confirmed that there have been a memory leak in VMExecutionContext::createFunction

code :
///////////////////////////////////////////////////////////////////////////////////////////// invokeFunc(&retval, unit->getMain(), Array::Create()); /////////////////////////////////////////////////////////////////////////////////////////////

I commented out, there is no problem(memery leak).

invokeFunc function in the create_function is to solve the fake main function,code format is as follow: ///////////////////////////////////////////////////////////////////////////////////////////// create_function('', '} echo "hi"; if (0) {'); /////////////////////////////////////////////////////////////////////////////////////////////

If there is no fake main function code, so comment out this( invokeFunc in createFunction call ) line also it doesn't matter.

But the ceateFunction functions are restricted.

I also in tracking invokeFunc memory leak problem, also ask the author to help track this problem together, there is progress I will keep in touch with you.Thanks.

scannell commented 11 years ago

@huzhiguang, thanks for the investigation. We don't use create_function much internally so this is a lower priority for us but we'll be happy to review a PR.

huzhiguang commented 11 years ago

I just fix a memery leak, create_function have two memery leaks. string_bin2hex function in f_create_function memery leak have fixed,but there is no solution to invokeFunc in create_function. Since we currently can't use create_function forge the main function, so I commented out invokeFunc function in create_function,but I go on traceing this problem(invokeFunc memery leakn in create_function).

scannell commented 11 years ago

I'll reopen this then. We'll be happy to take a pull if you find it. Thanks for the other fix.

scannell commented 10 years ago

Closing due to inactivity. If you provide more detailed information and/or a fix we'd be happy to take it, but we haven't seen this and don't use create_function() so we're not going to end up doing anything ourselves here.