...
Basically, we move a heap allocation elsewhere in the program. That heap allocation may walk the stack. Walking the stack may read fields w.r.t the inline call frame on the stack. However, by the time we actually walk the stack those fields may no longer be valid.
Some ways to solve this: 1. Update CallSiteIndex to be the forExit CodeOrigin instead of the semantic origin. This should mostly Just Work. Some potential fallout is a stack trace may give you unexpected line/column info. The reason this is nice is it may fix other future bugs that run into similar issues reading an inline call frame's stack slots. 2. Update OAS to not perform such moving of code when we detect this scenario. I kind of like this since LICM/ArgumentsElimination models this scenario correctly and won't hoist/eliminate code in such a situation. 3. All of these heap allocations that move should use a DeferGCForAWhile. I don't like this solution since it means you can write programs that allocate indefinitely without GCing. But it would work.
<rdar://problem/47280215>
Created attachment 359986 [details] patch
This patch goes with approach (2). I'm open to hearing cases of doing (1) or (3).
Comment on attachment 359986 [details] patch Attachment 359986 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/10868080 New failing tests: stress/object-allocation-sinking-phase-must-only-move-allocations-if-stack-trace-is-still-valid.js.default apiTests
(In reply to Build Bot from comment #6) > Comment on attachment 359986 [details] > patch > > Attachment 359986 [details] did not pass jsc-ews (mac): > Output: https://webkit-queues.webkit.org/results/10868080 > > New failing tests: > stress/object-allocation-sinking-phase-must-only-move-allocations-if-stack- > trace-is-still-valid.js.default > apiTests I'm not sure what's happening here. This passes for me locally as it's the test case I'm adding...
Comment on attachment 359986 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=359986&action=review > JSTests/stress/object-allocation-sinking-phase-must-only-move-allocations-if-stack-trace-is-still-valid.js:1 > +//@ runDefault("useConcurrentJIT=0", "jitPolicyScale=0", "collectContinuously=1") oops, I'm missing "--" before these options.
Created attachment 359991 [details] patch
Comment on attachment 359991 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=359991&action=review r=me > Source/JavaScriptCore/ChangeLog:11 > + walk the stack at the point in the program that it moved the allocation to. nit: *to* walk the ...
Created attachment 360027 [details] patch for landing
Comment on attachment 360027 [details] patch for landing Clearing flags on attachment: 360027 Committed r240447: <https://trac.webkit.org/changeset/240447>
All reviewed patches have been landed. Closing bug.