On the Windows build, the latest JSC (since before r156490) crashes when running testapi. The crash stack trace looks like this: JavaScriptCore.dll!WTF::Vector<wchar_t,64,WTF::UnsafeVectorOverflow>::size() Line 569 + 0x11 bytes C++ JavaScriptCore.dll!JSC::MarkedAllocator::forEachBlock<JSC::Free>(JSC::Free & functor) Line 141 + 0x8 bytes C++ JavaScriptCore.dll!JSC::MarkedSpace::forEachBlock<JSC::Free>(JSC::Free & functor) Line 230 C++ JavaScriptCore.dll!JSC::MarkedSpace::~MarkedSpace() Line 106 C++ JavaScriptCore.dll!JSC::Heap::~Heap() Line 282 + 0xee bytes C++ JavaScriptCore.dll!JSC::VM::~VM() Line 356 + 0x399 bytes C++ JavaScriptCore.dll!JSC::VM::`scalar deleting destructor'() + 0x16 bytes C++ JavaScriptCore.dll!WTF::ThreadSafeRefCounted<JSC::VM>::deref() Line 137 + 0x1c bytes C++ JavaScriptCore.dll!WTF::derefIfNotNull<JSC::VM>(JSC::VM * ptr) Line 45 C++ JavaScriptCore.dll!WTF::RefPtr<JSC::VM>::clear() Line 102 + 0x9 bytes C++ JavaScriptCore.dll!JSC::JSLockHolder::~JSLockHolder() Line 84 C++ JavaScriptCore.dll!JSGlobalContextRelease(OpaqueJSContext * ctx) Line 179 C++ testapi.exe!main(int argc, char * * argv) Line 1176 + 0xf bytes C++ testapi.exe!__tmainCRTStartup() Line 555 + 0x17 bytes C kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes This crash is reproducible every time we run testapi.exe.
That backtrace makes no sense. We don't access a Vector in MarkedAllocator::forEachBlock. Is this a debug backtrace?
(In reply to comment #1) > That backtrace makes no sense. We don't access a Vector in MarkedAllocator::forEachBlock. Is this a debug backtrace? This was run on a debug build. When it crashed, it gave me the opportunity to attach Visual Studio, and that's how I grabbed the stack trace.
Hmm, the only object I can find that uses a Vector<wchar_t,64,WTF::UnsafeVectorOverflow> is the JSStringBuilder. So maybe there's something weird going on there. Can you attach a full stack trace (e.g. with error code)?
(In reply to comment #3) > Hmm, the only object I can find that uses a Vector<wchar_t,64,WTF::UnsafeVectorOverflow> is the JSStringBuilder. So maybe there's something weird going on there. Can you attach a full stack trace (e.g. with error code)? That is the full stack trace.
> That is the full stack trace. There's no crash log anywhere? Not even something to tell you what happened (e.g. segfault, etc.)?
<rdar://problem/15211539>
Seems like the JSC::Free must have called a bad function pointer. My guess is that the top stack frame is just bogus -- a tools bug that happens when there's a PC in the backtrace that doesn't correspond to real code with debug info.
Created attachment 218294 [details] Patch
The reason for the crash is that the wrong memory block is decommitted. This can happen if no memory has been committed in the reserved block before the JSStack object is destroyed. In the JSStack destructor, the pointer to decommit then points to the end of the block (or the start of the next), and the decommit size is zero. If there is a block just after the block we are trying to decommit, this block will be decommitted, since Windows will decommit the whole block, if the decommit size is zero (see VirtualFree). When somebody tries to read/write to this block later, we crash.
Comment on attachment 218294 [details] Patch Clearing flags on attachment: 218294 Committed r160004: <http://trac.webkit.org/changeset/160004>
All reviewed patches have been landed. Closing bug.
(In reply to comment #9) > The reason for the crash is that the wrong memory block is decommitted. > This can happen if no memory has been committed in the reserved block before the JSStack object is destroyed. > In the JSStack destructor, the pointer to decommit then points to the end of the block (or the start of the next), and the decommit size is zero. > If there is a block just after the block we are trying to decommit, this block will be decommitted, since Windows will decommit the whole block, > if the decommit size is zero (see VirtualFree). When somebody tries to read/write to this block later, we crash. Nice work!
(In reply to comment #9) > The reason for the crash is that the wrong memory block is decommitted. > This can happen if no memory has been committed in the reserved block before the JSStack object is destroyed. > In the JSStack destructor, the pointer to decommit then points to the end of the block (or the start of the next), and the decommit size is zero. > If there is a block just after the block we are trying to decommit, this block will be decommitted, since Windows will decommit the whole block, > if the decommit size is zero (see VirtualFree). When somebody tries to read/write to this block later, we crash. Nice catch. However, I think the better fix would be to make OSAllocator::decommit() handle a 0 size as one would expect i.e. do nothing. The behavior of Window's VirtualFree() in terms of its interpretation of what a 0 size means is not intuitive. I'll look into fixing this in separate patch + bug. Fixing the interpretation of size 0 in OSAllocator will also help avoid future surprises like this from manifesting.
Reopening this bug to apply the better fix in OSAllocatorWin.
Created attachment 218367 [details] Fixing OSAllocatorWin instead.
Comment on attachment 218367 [details] Fixing OSAllocatorWin instead. r=me
Comment on attachment 218367 [details] Fixing OSAllocatorWin instead. View in context: https://bugs.webkit.org/attachment.cgi?id=218367&action=review > Source/WTF/wtf/OSAllocatorWin.cpp:69 > + return; // Nothing to do. This comment seems unnecessary. > Source/WTF/wtf/OSAllocatorWin.cpp:78 > + return; // Nothing to do. Ditto.
Comment on attachment 218367 [details] Fixing OSAllocatorWin instead. View in context: https://bugs.webkit.org/attachment.cgi?id=218367&action=review >> Source/WTF/wtf/OSAllocatorWin.cpp:69 >> + return; // Nothing to do. > > This comment seems unnecessary. Instead, I would comment on how Windows does odd things when the size is 0 and refer them to this bug.
Comment on attachment 218367 [details] Fixing OSAllocatorWin instead. I'll update the comments based on Mark H's suggestion and land the patch manually.
Update patch landed in r160063: <http://trac.webkit.org/r160063>.