In one bizarre case, B3 used a callee-save for the CodeBlock constant created in the prologue before the stack overflow check.
Created attachment 310890 [details] the patch
Comment on attachment 310890 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=310890&action=review > Source/JavaScriptCore/heap/Subspace.cpp:56 > + > + I'll revert
Comment on attachment 310890 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=310890&action=review r=me with comment. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:233 > + // OOPS! Need bug URL. Please add bug url before landing. > Source/JavaScriptCore/heap/Subspace.cpp:220 > +ALWAYS_INLINE void Subspace::didAllocate(void* ptr) > +{ > + UNUSED_PARAM(ptr); > + > + // This is useful for logging allocations, or doing other kinds of debugging hacks. Just make > + // sure you JSC_forceGCSlowPaths=true. > +} > + This set of changes (while useful) seems totally unrelated to this patch. Did you intend to include it?
(In reply to Mark Lam from comment #3) > Comment on attachment 310890 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310890&action=review > > r=me with comment. > > > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:233 > > + // OOPS! Need bug URL. > > Please add bug url before landing. Ooops! I filed it: https://bugs.webkit.org/show_bug.cgi?id=172456 > > > Source/JavaScriptCore/heap/Subspace.cpp:220 > > +ALWAYS_INLINE void Subspace::didAllocate(void* ptr) > > +{ > > + UNUSED_PARAM(ptr); > > + > > + // This is useful for logging allocations, or doing other kinds of debugging hacks. Just make > > + // sure you JSC_forceGCSlowPaths=true. > > +} > > + > > This set of changes (while useful) seems totally unrelated to this patch. > Did you intend to include it? Yeah. This bug looked like a GC bug until for the first ~2 hours that I spent on it. Every time I run into a GC bug, the first thing I do is instrument every allocation site. I finally decided to do it the "right" way.
Landed in http://trac.webkit.org/changeset/217221/webkit
Comment on attachment 310890 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=310890&action=review > Source/JavaScriptCore/ChangeLog:10 > + The FTL needs to run B3's callee-save register restoration before it runs the exception > + handler's callee-save register restoration. This exposes B3's callee-save register > + algorithm in AssemblyHelpers so that the FTL can call it. Nice. I actually realized this bug when reading the FTL overflow code when doing wasm overflow. I should have file a bug when I thought this could be an issue. An alternative way to solve this, and is what I did for wasm, which is unwind from the top frame (not the caller frame of the top frame), and just let the unwinder restore the callee saves. I concluded this was safe for wasm/JS because the caller is responsible for ensuring that the callee's FP is not overflowed.