Last Comment Bug 655368 - Allocator mismatch: js::WeakMap::finalize uses "delete" on table allocated with "malloc"
: Allocator mismatch: js::WeakMap::finalize uses "delete" on table allocated wi...
Status: RESOLVED FIXED
fixed-in-tracemonkey comment 2 explai...
: regression, testcase, valgrind
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: David Mandelin [:dmandelin]
:
:
Mentors:
Depends on:
Blocks: 326633
  Show dependency treegraph
 
Reported: 2011-05-06 14:27 PDT by Jesse Ruderman
Modified: 2011-08-15 06:40 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Patch (978 bytes, patch)
2011-05-11 09:28 PDT, David Mandelin [:dmandelin]
gal: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-05-06 14:27:41 PDT
var a = new WeakMap();
var b = new WeakMap();
a.set(b, null);


valgrind ./js q.js

> ==13533== Mismatched free() / delete / delete []
> ==13533==    at 0x4C2806F: operator delete(void*) (vg_replace_malloc.c:387)
> ==13533==    by 0x592892: js::WeakMap::finalize(JSContext*, JSObject*) (jsweakmap.cpp:300)
> ==13533==    by 0x4AC5A8: JSObject::finalize(JSContext*) (jsobjinlines.h:141)
> ==13533==    by 0x4B5CDE: js::gc::Arena<JSObject>::finalize(JSContext*) (jsgc.cpp:231)
> ==13533==    by 0x4AB442: void js::gc::FinalizeArenas<JSObject>(JSContext*, js::gc::ArenaHeader**) (jsgc.cpp:278)
> ==13533==    by 0x4B0D2E: void js::gc::ArenaList::finalizeNow<JSObject>(JSContext*) (jsgc.cpp:1231)
> ==13533==    by 0x4A9A86: JSCompartment::finalizeObjectArenaLists(JSContext*) (jsgc.cpp:2016)
> ==13533==    by 0x4AA491: MarkAndSweep(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2404)
> ==13533==    by 0x4AA802: GCCycle(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2682)
> ==13533==    by 0x4AA9D8: js_GC(JSContext*, JSCompartment*, JSGCInvocationKind) (jsgc.cpp:2753)
> ==13533==    by 0x45D9F3: js_DestroyContext(JSContext*, JSDestroyContextMode) (jscntxt.cpp:643)
> ==13533==    by 0x4278F7: JS_DestroyContext (jsapi.cpp:1032)
> ==13533==  Address 0x5eb71f0 is 0 bytes inside a block of size 104 alloc'd
> ==13533==    at 0x4C2901C: malloc (vg_replace_malloc.c:236)
> ==13533==    by 0x417F29: js_malloc (jsutil.h:239)
> ==13533==    by 0x435B4A: JSRuntime::malloc_(unsigned long, JSContext*) (jscntxt.h:724)
> ==13533==    by 0x436026: JSContext::malloc_(unsigned long) (jscntxt.h:1283)
> ==13533==    by 0x592BA1: js::WeakMap* JSContext::new_<js::WeakMap, JSContext*>(JSContext* const&) (in /home/jruderman/tm-dbg-vg/shell/js)
> ==13533==    by 0x592419: js::WeakMap::set(JSContext*, unsigned int, js::Value*) (jsweakmap.cpp:204)
> ==13533==    by 0x4C75C9: js::CallJSNative(JSContext*, int (*)(JSContext*, unsigned int, js::Value*), unsigned int, js::Value*) (jscntxtinlines.h:277)
> ==13533==    by 0x6F075A: js::Interpret(JSContext*, js::StackFrame*, unsigned int, js::InterpMode) (jsinterp.cpp:4664)
> ==13533==    by 0x4C2C0F: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:603)
> ==13533==    by 0x4C3FB9: js::Execute(JSContext*, JSObject&, JSScript*, js::StackFrame*, unsigned int, js::Value*) (jsinterp.cpp:964)
> ==13533==    by 0x43113F: JS_ExecuteScript (jsapi.cpp:4940)
> ==13533==    by 0x4051D9: Process(JSContext*, JSObject*, char*, int) (js.cpp:451)
Comment 1 Jesse Ruderman 2011-05-06 14:30:04 PDT
There are several other "delete table;" lines in jsweakmap.cpp, fwiw.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-05-06 19:13:38 PDT
To be clear, I requested tracking because this looks like a memory-safety bug in a new feature...
Comment 3 David Mandelin [:dmandelin] 2011-05-11 09:28:57 PDT
Created attachment 531655 [details] [diff] [review]
Patch
Comment 4 Andreas Gal :gal 2011-05-24 14:55:53 PDT
bz, this is not a bug with our current allocator on aurora. Do you agree?
Comment 5 David Mandelin [:dmandelin] 2011-05-24 15:08:25 PDT
http://hg.mozilla.org/tracemonkey/rev/4d3b321e2559
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2011-05-25 22:25:47 PDT
I don't know... esp. for Linux distros that build with a separate (system) Spidermonkey.
Comment 7 JP Rosevear [:jpr] 2011-06-10 07:06:33 PDT
If the fix is safe and standalone, you can just make an approval request David.  Or if you believe it doesn't affect aurora (#3), just change the status to unaffected for 6.
Comment 8 David Mandelin [:dmandelin] 2011-06-24 11:58:39 PDT
http://hg.mozilla.org/mozilla-aurora/rev/bab230791c61
Comment 9 Chris Leary [:cdleary] (not checking bugmail) 2011-07-05 22:01:14 PDT
http://hg.mozilla.org/mozilla-central/rev/4d3b321e2559
Comment 10 AndreiD[QA] 2011-08-12 09:06:54 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20100101 Firefox/6.0

I was trying to see if this is fixed for Fx6 since the flag "status-firefox6" is set on "fixed", but I couldn't.
Is there a test case or any steps / guidelines for this bug that can be used to verify the fix? Thanks
Comment 11 Jesse Ruderman 2011-08-15 06:40:35 PDT
AndreiD, comment 0 contains a testcase and steps to reproduce. You'll need to install valgrind (a memory debugger) and then build a js shell with "./configure --enable-valgrind".

http://valgrind.org/
https://developer.mozilla.org/En/SpiderMonkey/Build_Documentation

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