Last Comment Bug 736679 - IonMonkey: Valgrind detects leak at Malloc (36 bytes in 1 blocks are definitely lost)
: IonMonkey: Valgrind detects leak at Malloc (36 bytes in 1 blocks are definite...
Status: RESOLVED FIXED
: mlk, testcase, valgrind
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Kannan Vijayan [:djvj]
:
Mentors:
Depends on:
Blocks: jsfunfuzz IonFuzz
  Show dependency treegraph
 
Reported: 2012-03-16 18:05 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-05-23 11:36 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.72 KB, text/plain)
2012-03-16 18:05 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Valgrind stack (2.53 KB, text/plain)
2012-03-16 18:05 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
Patch (1.45 KB, patch)
2012-05-23 11:25 PDT, Kannan Vijayan [:djvj]
nicolas.b.pierron: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-03-16 18:05:21 PDT
Created attachment 606813 [details]
testcase

The attached testcase throws up a Valgrind error in js debug shell on IonMonkey changeset b96587f04076 with -m, -a, --ion and -n.

==16007== 376 bytes in 1 blocks are definitely lost in loss record 613 of 694
==16007==    at 0xC743: malloc (vg_replace_malloc.c:266)
==16007==    by 0x10022F1FD: js::ion::IonScript::New(JSContext*, unsigned int, unsigned int, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long) (Utility.h:173)
==16007==    by 0x100221AC5: js::ion::CodeGenerator::generate() (CodeGenerator.cpp:1701)
==16007==    by 0x1002309FF: TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&) (Ion.cpp:734)
==16007==    by 0x10022FB02: Compile(JSContext*, JSScript*, js::StackFrame*, unsigned char*) (Ion.cpp:768)
==16007==    by 0x10022FDFF: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*) (Ion.cpp:924)
==16007==    by 0x100075C37: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2807)
==16007==    by 0x100181F2C: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1079)
==16007==    by 0x10006E44A: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:477)
==16007==    by 0x10007B15D: js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) (jsinterp.cpp:679)
==16007==    by 0x10007B2C6: js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) (jsinterp.cpp:720)
==16007==    by 0x100018F4A: JS_ExecuteScript (jsapi.cpp:5296)
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2012-03-16 18:05:42 PDT
Created attachment 606814 [details]
Valgrind stack
Comment 2 David Anderson [:dvander] 2012-05-18 12:21:24 PDT
Gary, I can't reproduce this anymore - can you?
Comment 3 Gary Kwong [:gkw] [:nth10sd] 2012-05-18 13:23:23 PDT
(In reply to David Anderson [:dvander] from comment #2)
> Gary, I can't reproduce this anymore - can you?

Yes, I still can, with -a on 32-bit debug js shell on Mac OS X 10.7 and changeset 47b283284868:

==43801== 36 bytes in 1 blocks are definitely lost in loss record 105 of 221
==43801==    at 0xF6C785: malloc (vg_replace_malloc.c:271)
==43801==    by 0x2356D: js_malloc (Utility.h:184)
==43801==    by 0x2BAF0: JSRuntime::malloc_(unsigned long, JSContext*) (jscntxt.h:861)
==43801==    by 0x259F9: JSContext::malloc_(unsigned long) (jscntxt.h:1298)
==43801==    by 0x5A8906: js::ion::BailoutClosure* JSContext::new_<js::ion::BailoutClosure>() (jscntxt.h:1323)
==43801==    by 0x5A545A: ConvertFrames(JSContext*, js::ion::IonActivation*, js::ion::IonBailoutIterator&) (Bailouts.cpp:239)
==43801==    by 0x5A529B: js::ion::Bailout(js::ion::BailoutStack*) (Bailouts.cpp:363)
==43801==    by 0x3A2BA02: ???
==43801==    by 0x5E964E: EnterIon(JSContext*, js::StackFrame*, void*) (Ion.cpp:991)
==43801==    by 0x5E922D: js::ion::Cannon(JSContext*, js::StackFrame*) (Ion.cpp:1021)
==43801==    by 0x17E87E: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:475)
==43801==    by 0x19F03E: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:558)
Comment 4 Kannan Vijayan [:djvj] 2012-05-23 09:40:04 PDT
I can repro this on 32-bit linux.  Looking into it.
Comment 5 Kannan Vijayan [:djvj] 2012-05-23 11:25:18 PDT
Created attachment 626521 [details] [diff] [review]
Patch

There are two paths in ThunkToInterpreter that forget to delete the BailoutClosure.
Comment 6 Kannan Vijayan [:djvj] 2012-05-23 11:34:57 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/85ad9bc4ea65

Should be good to close, waiting for verification.

Note You need to log in before you can comment on or make changes to this bug.