IonMonkey: Valgrind detects leak at Malloc (36 bytes in 1 blocks are definitely lost)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gkw, Assigned: djvj)

Tracking

(Blocks: 2 bugs, {mlk, testcase, valgrind})

Other Branch
x86
Mac OS X
mlk, testcase, valgrind
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
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)
(Reporter)

Comment 1

6 years ago
Created attachment 606814 [details]
Valgrind stack
Version: Trunk → Other Branch
Gary, I can't reproduce this anymore - can you?
(Reporter)

Comment 3

5 years ago
(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)
Summary: IonMonkey: Valgrind detects leak at Malloc, js::ion::IonScript::New (376 bytes in 1 blocks are definitely lost) → IonMonkey: Valgrind detects leak at Malloc (36 bytes in 1 blocks are definitely lost)
(Assignee)

Comment 4

5 years ago
I can repro this on 32-bit linux.  Looking into it.
(Assignee)

Updated

5 years ago
Assignee: general → kvijayan
(Assignee)

Comment 5

5 years ago
Created attachment 626521 [details] [diff] [review]
Patch

There are two paths in ThunkToInterpreter that forget to delete the BailoutClosure.
Attachment #626521 - Flags: review?(nicolas.b.pierron)
Attachment #626521 - Flags: review?(nicolas.b.pierron) → review+
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/85ad9bc4ea65

Should be good to close, waiting for verification.
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.