Last Comment Bug 718053 - 1,104 or 1,932 bytes in 92 blocks are definitely lost in loss record 1 of 1 as detected by Valgrind
: 1,104 or 1,932 bytes in 92 blocks are definitely lost in loss record 1 of 1 a...
Status: RESOLVED FIXED
[MemShrink:P2]
: testcase, valgrind
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical with 1 vote (vote)
: mozilla13
Assigned To: [PTO to Dec5] Bill McCloskey (:billm)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 661903
Blocks: jsfunfuzz 728779
  Show dependency treegraph
 
Reported: 2012-01-13 13:56 PST by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-03-14 15:34 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
sort-of reduced testcase (11.68 KB, text/plain)
2012-01-13 13:56 PST, Gary Kwong [:gkw] [:nth10sd]
no flags Details
another sort-of reduced testcase (12.01 KB, text/plain)
2012-01-13 14:01 PST, Gary Kwong [:gkw] [:nth10sd]
no flags Details
remove unused JSRuntime fields (1.44 KB, patch)
2012-01-17 16:13 PST, Nicholas Nethercote [:njn]
wmccloskey: review+
Details | Diff | Splinter Review
patch for unrelated leaks (1.37 KB, patch)
2012-02-17 17:25 PST, [PTO to Dec5] Bill McCloskey (:billm)
cdleary: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-01-13 13:56:11 PST
Created attachment 588518 [details]
sort-of reduced testcase

Attached testcase shows an error with js opt builds on m-c changeset 8ffdb4c7404a when run with -m -a.

valgrind --smc-check=all-non-file --leak-check=full ./js -m -a w6-orig.js

===

==26243== 1,104 bytes in 92 blocks are definitely lost in loss record 1 of 1
==26243==    at 0x4C29313: malloc (vg_replace_malloc.c:263)
==26243==    by 0x505682: SaveScriptFilename(JSContext*, char const*) (Utility.h:135)
==26243==    by 0x506CAC: JSScript::NewScriptFromEmitter(JSContext*, js::BytecodeEmitter*) (jsscript.cpp:1155)
==26243==    by 0x57E5C2: js::frontend::CompileScript(JSContext*, JSObject*, js::StackFrame*, JSPrincipals*, JSPrincipals*, unsigned int, unsigned short const*, unsigned long, char const*, unsigned int, JSVersion, JSString*, unsigned int) (BytecodeCompiler.cpp:361)
==26243==    by 0x41B4E9: JS_EvaluateUCScriptForPrincipals (jsapi.cpp:5339)
==26243==    by 0x41B744: JS_EvaluateUCScript (jsapi.cpp:5390)
==26243==    by 0x4058D5: EvalInContext(JSContext*, unsigned int, JS::Value*) (js.cpp:3017)
==26243==    by 0x405B3A5: ???
==26243==    by 0x5C9AC6: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1051)
==26243==    by 0x5C9E2F: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1109)
==26243==    by 0x498DD8: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:3050)
==26243==    by 0x5C9B02: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1078)
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2012-01-13 14:01:01 PST
Created attachment 588520 [details]
another sort-of reduced testcase

==26674== 1,932 bytes in 92 blocks are definitely lost in loss record 1 of 1
==26674==    at 0x4C29313: malloc (vg_replace_malloc.c:263)
==26674==    by 0x505682: SaveScriptFilename(JSContext*, char const*) (Utility.h:135)
==26674==    by 0x506CAC: JSScript::NewScriptFromEmitter(JSContext*, js::BytecodeEmitter*) (jsscript.cpp:1155)
==26674==    by 0x57E5C2: js::frontend::CompileScript(JSContext*, JSObject*, js::StackFrame*, JSPrincipals*, JSPrincipals*, unsigned int, unsigned short const*, unsigned long, char const*, unsigned int, JSVersion, JSString*, unsigned int) (BytecodeCompiler.cpp:361)
==26674==    by 0x41B4E9: JS_EvaluateUCScriptForPrincipals (jsapi.cpp:5339)
==26674==    by 0x41B744: JS_EvaluateUCScript (jsapi.cpp:5390)
==26674==    by 0x4058D5: EvalInContext(JSContext*, unsigned int, JS::Value*) (js.cpp:3017)
==26674==    by 0x405B3A5: ???
==26674==    by 0x5C9AC6: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1051)
==26674==    by 0x5C9E2F: js::mjit::JaegerShot(JSContext*, bool) (MethodJIT.cpp:1109)
==26674==    by 0x498DD8: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:3050)
==26674==    by 0x5C9B02: js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool) (MethodJIT.cpp:1078)
==26674== 
==26674== LEAK SUMMARY:
==26674==    definitely lost: 1,932 bytes in 92 blocks
==26674==    indirectly lost: 0 bytes in 0 blocks
==26674==      possibly lost: 0 bytes in 0 blocks
==26674==    still reachable: 0 bytes in 0 blocks
==26674==         suppressed: 0 bytes in 0 blocks

Tested on Ubuntu Linux 64-bit.
Comment 2 Nicholas Nethercote [:njn] 2012-01-17 16:12:17 PST
I can reproduce.  billm, this looks like yours.
Comment 3 Nicholas Nethercote [:njn] 2012-01-17 16:13:45 PST
Created attachment 589336 [details] [diff] [review]
remove unused JSRuntime fields

Just a clean-up while I was in the neighbourhood:  JSRuntime has a couple of unused members left over from bug 661903.
Comment 4 [PTO to Dec5] Bill McCloskey (:billm) 2012-02-01 17:06:28 PST
Comment on attachment 589336 [details] [diff] [review]
remove unused JSRuntime fields

I'll try to get a chance to look at this soon.
Comment 5 Nicholas Nethercote [:njn] 2012-02-02 14:34:26 PST
Comment on attachment 589336 [details] [diff] [review]
remove unused JSRuntime fields

These fields were removed elsewhere.  I'm guessing Igor got them.
Comment 6 Igor Bukanov 2012-02-02 16:04:32 PST
(In reply to Nicholas Nethercote [:njn] from comment #5)
> Comment on attachment 589336 [details] [diff] [review]
> remove unused JSRuntime fields
> 
> These fields were removed elsewhere.  I'm guessing Igor got them.

That was Luke :)
Comment 7 [PTO to Dec5] Bill McCloskey (:billm) 2012-02-17 17:25:13 PST
Created attachment 598453 [details] [diff] [review]
patch for unrelated leaks

I'm not sure why, but I can't reproduce this problem. When I run it on the given changeset, it says that no leaks are possible. Nick, could you maybe look into this a little more, since you're able to reproduce it? Normally, script filenames are marked via their JSScript and then swept out of the table and freed. If they're not getting freed, that suggests that there are scripts still alive when the last GC happens. That's not supposed to happen.

I did find a leak with this test case on tip. I also fixed an uninitialized memory read.
Comment 8 [PTO to Dec5] Bill McCloskey (:billm) 2012-02-21 09:51:41 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/323b7bb0129e

I think we want to leave this open to address the original problem with this test case.
Comment 9 Ed Morley [:emorley] 2012-02-22 10:50:03 PST
https://hg.mozilla.org/mozilla-central/rev/323b7bb0129e
Comment 10 Serge Gautherie (:sgautherie) 2012-02-26 00:35:05 PST
(In reply to Ed Morley [:edmorley] from comment #9)
> https://hg.mozilla.org/mozilla-central/rev/323b7bb0129e

Ftr, this fixed bug 728779.
Comment 11 Gary Kwong [:gkw] [:nth10sd] 2012-03-14 15:31:07 PDT
(In reply to Bill McCloskey (:billm) from comment #8)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/323b7bb0129e
> 
> I think we want to leave this open to address the original problem with this
> test case.

The original problem with this testcase is now gone.

I'll assume that the seemingly-unrelated patch in comment 7 somehow fixed the problem.

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