Closed Bug 718053 Opened 14 years ago Closed 13 years ago

1,104 or 1,932 bytes in 92 blocks are definitely lost in loss record 1 of 1 as detected by Valgrind

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: gkw, Assigned: billm)

References

Details

(Keywords: testcase, valgrind, Whiteboard: [MemShrink:P2] )

Attachments

(3 files, 1 obsolete file)

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)
==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.
Summary: 1,104 bytes in 92 blocks are definitely lost in loss record 1 of 1 → 1,104 or 1,932 bytes in 92 blocks are definitely lost in loss record 1 of 1 as detected by Valgrind
Whiteboard: [MemShrink]
Whiteboard: [MemShrink] → [MemShrink:P2]
I can reproduce. billm, this looks like yours.
Assignee: general → wmccloskey
Depends on: 661903
Attached patch remove unused JSRuntime fields (obsolete) — Splinter Review
Just a clean-up while I was in the neighbourhood: JSRuntime has a couple of unused members left over from bug 661903.
Attachment #589336 - Flags: review?(wmccloskey)
Comment on attachment 589336 [details] [diff] [review] remove unused JSRuntime fields I'll try to get a chance to look at this soon.
Attachment #589336 - Flags: review?(wmccloskey) → review+
Comment on attachment 589336 [details] [diff] [review] remove unused JSRuntime fields These fields were removed elsewhere. I'm guessing Igor got them.
Attachment #589336 - Attachment is obsolete: true
(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 :)
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.
Attachment #598453 - Flags: review?(christopher.leary)
Attachment #598453 - Flags: review?(christopher.leary) → review+
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.
Whiteboard: [MemShrink:P2] → [MemShrink:P2] [leave-open]
(In reply to Ed Morley [:edmorley] from comment #9) > https://hg.mozilla.org/mozilla-central/rev/323b7bb0129e Ftr, this fixed bug 728779.
(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.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P2] [leave-open] → [MemShrink:P2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: