180653 2017-12-11 10:00:52 -0800 LLInt: reserve 16 bytes of stack on MIPS for native calls 2017-12-12 11:03:39 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Other Linux RESOLVED FIXED InRadar P2 Normal --- 1 guijemont webkit-unassigned aperez clopez commit-queue ews-watchlist keith_miller mark.lam msaboff saam webkit-bug-importer oldest_to_newest 1380254 0 guijemont 2017-12-11 10:00:52 -0800 The calling convention on MIPS is that the caller reserves 16 bytes (4 words) of stack space for all calls (regardless of the number of arguments). The callee can use that space. in LLInt's nativeCallTrampoline (which is only used if JIT is disabled), we effectively only save 8 bytes when adjusting the alignment. This crashes many date-related tests when running in llint-only mode when compiling in -Os. 1380256 1 guijemont 2017-12-11 10:02:41 -0800 Note that it does not crash when the JIT is enabled, because it seems we use the thunk generated by JSC::nativeForGenerator() even when calling from llint. This thunk correctly allocates the 16 bytes of stack. 1380265 2 328994 guijemont 2017-12-11 10:07:09 -0800 Created attachment 328994 Patch Patch fixing the issue. 1380342 3 328994 aperez 2017-12-11 12:23:23 -0800 Comment on attachment 328994 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328994&action=review Informal review: r+, with a comment. Please check it before landing. > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2086 > + # calling convention says to save stack space for 4 first registers in Wow, good find. A similar scheme is used in the “internalFunctionCallTrampoline()” macro but this patch does NOT modify it accordingly. Is that intended? 1380350 4 guijemont 2017-12-11 12:48:53 -0800 > > Wow, good find. A similar scheme is used in the > “internalFunctionCallTrampoline()” macro but this patch does NOT > modify it accordingly. Is that intended? My understanding is that nativeCallTrampoline() is used to call C++ (native) functions, and internalFunctionCallTrampoline() is used to call js (internal) functions. Neither LLInt nor the JIT use that extra space, so we can save some memory by not allocating it. 1380382 5 aperez 2017-12-11 13:54:41 -0800 (In reply to Guillaume Emont from comment #4) > > > > Wow, good find. A similar scheme is used in the > > “internalFunctionCallTrampoline()” macro but this patch does NOT > > modify it accordingly. Is that intended? > > My understanding is that nativeCallTrampoline() is used to call C++ (native) > functions, and internalFunctionCallTrampoline() is used to call js > (internal) functions. Neither LLInt nor the JIT use that extra space, so we > can save some memory by not allocating it. That was also my suspicion, okay then! 1380663 6 328994 clopez 2017-12-12 08:48:39 -0800 Comment on attachment 328994 Patch r=me (trusting Adrian review) 1380709 7 328994 commit-queue 2017-12-12 10:40:33 -0800 Comment on attachment 328994 Patch Clearing flags on attachment: 328994 Committed r225788: <https://trac.webkit.org/changeset/225788> 1380710 8 commit-queue 2017-12-12 10:40:35 -0800 All reviewed patches have been landed. Closing bug. 1380726 9 webkit-bug-importer 2017-12-12 11:03:39 -0800 <rdar://problem/35998684> 328994 2017-12-11 10:07:09 -0800 2017-12-12 10:40:33 -0800 Patch bug-180653-20171211120708.patch text/plain 2186 guijemont U3VidmVyc2lvbiBSZXZpc2lvbjogMjI1NzQyCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBl MGM2MmFkY2E2ZmI3YjA5YmZmNWMyMTY0NzQyYWUzYTdiMTM5NTNhLi4wNDg0Yzk0NTZhZjQxNDg5 ZjRmNGY2ZWNmNTA0ZmE4NjY4YTZlNTllIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxNy0xMi0xMSAgR3VpbGxhdW1lIEVtb250ICA8Z3VpamVtb250QGlnYWxp YS5jb20+CisKKyAgICAgICAgTExJbnQ6IHJlc2VydmUgMTYgYnl0ZXMgb2Ygc3RhY2sgb24gTUlQ UyBmb3IgbmF0aXZlIGNhbGxzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xODA2NTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIGxsaW50L0xvd0xldmVsSW50ZXJwcmV0ZXIzMl82NC5hc206CisgICAgICAg IE9uIE1JUFMsIHN1YnN0cmFjdCAyNCBmcm9tIHRoZSBzdGFjayBwb2ludGVyICgxNiBmb3IgY2Fs bGluZworICAgICAgICBjb252ZW50aW9uICsgOCB0byBiZSAxNi1hbGlnbmVkKSBpbnN0ZWFkIG9m IHRoZSA4IG9uIG90aGVyIHBsYXRmb3JtcworICAgICAgICAoZm9yIGFsaWdubWVudCkuCisKIDIw MTctMTItMTAgIEZpbGlwIFBpemxvICA8ZnBpemxvQGFwcGxlLmNvbT4KIAogICAgICAgICBIYXJk ZW4gYSBmZXcgYXNzZXJ0aW9ucyBpbiBHQyBzd2VlcApkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFT Y3JpcHRDb3JlL2xsaW50L0xvd0xldmVsSW50ZXJwcmV0ZXIzMl82NC5hc20gYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjMyXzY0LmFzbQppbmRleCA3ZDYy NjljYjczNjdkNzM4MDkzNmJkNTYwYzg4ZTUwMjAzNWJjNWU4Li5hN2E0NTdjMTI3NzM1OTNhNDI0 NDBjNTgxY2I4NjE4N2MxMDA1MWJhIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv bGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjMyXzY0LmFzbQorKysgYi9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvbGxpbnQvTG93TGV2ZWxJbnRlcnByZXRlcjMyXzY0LmFzbQpAQCAtMjA4Miw3ICsyMDgy LDE0IEBAIG1hY3JvIG5hdGl2ZUNhbGxUcmFtcG9saW5lKGV4ZWN1dGFibGVPZmZzZXRUb0Z1bmN0 aW9uKQogICAgICAgICBsb2FkcCBNYXJrZWRCbG9jazo6bV92bVt0M10sIHQzCiAgICAgICAgIGFk ZHAgOCwgc3AKICAgICBlbHNpZiBBUk0gb3IgQVJNdjcgb3IgQVJNdjdfVFJBRElUSU9OQUwgb3Ig Q19MT09QIG9yIE1JUFMKLSAgICAgICAgc3VicCA4LCBzcCAjIGFsaWduIHN0YWNrIHBvaW50ZXIK KyAgICAgICAgaWYgTUlQUworICAgICAgICAjIGNhbGxpbmcgY29udmVudGlvbiBzYXlzIHRvIHNh dmUgc3RhY2sgc3BhY2UgZm9yIDQgZmlyc3QgcmVnaXN0ZXJzIGluCisgICAgICAgICMgYWxsIGNh c2VzLiBUbyBtYXRjaCBvdXIgMTYtYnl0ZSBhbGlnbm1lbnQsIHRoYXQgbWVhbnMgd2UgbmVlZCB0 bworICAgICAgICAjIHRha2UgMjQgYnl0ZXMKKyAgICAgICAgICAgIHN1YnAgMjQsIHNwCisgICAg ICAgIGVsc2UKKyAgICAgICAgICAgIHN1YnAgOCwgc3AgIyBhbGlnbiBzdGFjayBwb2ludGVyCisg ICAgICAgIGVuZAogICAgICAgICAjIHQxIGFscmVhZHkgY29udGFpbnMgdGhlIENhbGxlZS4KICAg ICAgICAgYW5kcCBNYXJrZWRCbG9ja01hc2ssIHQxCiAgICAgICAgIGxvYWRwIE1hcmtlZEJsb2Nr OjptX3ZtW3QxXSwgdDEKQEAgLTIwOTksNyArMjEwNiwxMSBAQCBtYWNybyBuYXRpdmVDYWxsVHJh bXBvbGluZShleGVjdXRhYmxlT2Zmc2V0VG9GdW5jdGlvbikKICAgICAgICAgbG9hZHAgQ2FsbGVl ICsgUGF5bG9hZE9mZnNldFtjZnJdLCB0MwogICAgICAgICBhbmRwIE1hcmtlZEJsb2NrTWFzaywg dDMKICAgICAgICAgbG9hZHAgTWFya2VkQmxvY2s6Om1fdm1bdDNdLCB0MwotICAgICAgICBhZGRw IDgsIHNwCisgICAgICAgIGlmIE1JUFMKKyAgICAgICAgICAgIGFkZHAgMjQsIHNwCisgICAgICAg IGVsc2UKKyAgICAgICAgICAgIGFkZHAgOCwgc3AKKyAgICAgICAgZW5kCiAgICAgZWxzZQogICAg ICAgICBlcnJvcgogICAgIGVuZAo=