RESOLVED FIXED Bug 127902
Fix the remaining regression caused by the jsCStack branch merge on Linux platforms 
https://bugs.webkit.org/show_bug.cgi?id=127902
Summary Fix the remaining regression caused by the jsCStack branch merge on Linux pla...
Csaba Osztrogonác
Reported 2014-01-30 03:09:33 PST
After https://bugs.webkit.org/show_bug.cgi?id=127898 there are still 2 failing jsc stress test: ** The following JSC stress test failures have been introduced: jsc-layout-tests.yaml/js/script-tests/function-apply-many-args.js.layout-dfg-eager-no-cjit jsc-layout-tests.yaml/js/script-tests/function-apply-many-args.js.layout-no-llint Results for JSC stress tests: 2 failures found.
Attachments
Debugging data - info proc all, instructions, registers with ToT (18.74 KB, text/plain)
2014-02-25 16:39 PST, Zan Dobersek 		no flags
Details
Patch (4.72 KB, patch)
2014-05-19 10:00 PDT, Zan Dobersek 		msaboff: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2014-01-31 11:08:58 PST
I only get useless backtraces when debugging this, even in debug builds. Not even anything disassemble-able.
Zan Dobersek
Comment 2 2014-02-03 02:16:54 PST
The problem appears to be in the JIT-compiled code for 'function g() { return f.apply(null, arguments); }'. Here's the backtrace: #0 0x00007fffb2b0fabe in ?? () #1 0x00007ffff7ecd170 in ?? () #2 0x00007ffff7ecd170 in ?? () #3 0x00007ffff7eafd90 in ?? () #4 0x0000000000442960 in ?? () #5 0x00007fffffffa0b0 in ?? () #6 0x0000000000000000 in ?? () Registers: rax 0x10001 65537 rbx 0x0 0 rcx 0x7ffffff7a050 140737487806544 rdx 0x7fffffef9fe0 140737487282144 rsi 0x7ffffff7a050 140737487806544 rdi 0x54fb00 5569280 rbp 0x7ffffff7a050 0x7ffffff7a050 rsp 0x7ffffff79fe0 0x7ffffff79fe0 r8 0x0 0 r9 0x101010101010101 72340172838076673 r10 0x7ffff747bd30 140737342061872 r11 0x44e828 4515880 r12 0x7ffff7eaff40 140737352761152 r13 0x7ffff7f0ffd0 140737353154512 r14 0xffff000000000000 -281474976710656 r15 0xffff000000000002 -281474976710654 rip 0x7fffb2b0fabe 0x7fffb2b0fabe eflags 0x10287 [ CF PF SF IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x0 0 es 0x0 0 fs 0x0 0 gs 0x0 0
Zan Dobersek
Comment 3 2014-02-03 02:19:18 PST
Here's the disassembled code, with the point of crash highlighted. Generated Baseline JIT code for g#DSCS4i:[0x54e2a0->0x7fffb0ade070, BaselineFunctionCall, 67], instruction count = 67 Source: function g() { return f.apply(null, arguments); } Code at [0x7fffb2b0f8c0, 0x7fffb2b0feac): 0x7fffb2b0f8c0: push %rbp 0x7fffb2b0f8c1: mov %rsp, %rbp 0x7fffb2b0f8c4: mov $0x54e2a0, %r11 0x7fffb2b0f8ce: mov %r11, 0x10(%rbp) 0x7fffb2b0f8d2: mov 0x30(%rbp), %rax 0x7fffb2b0f8d6: mov %rax, 0x4fbf90 0x7fffb2b0f8e0: lea -0x70(%rbp), %rdx 0x7fffb2b0f8e4: mov $0x44e828, %r11 0x7fffb2b0f8ee: cmp %rdx, (%r11) 0x7fffb2b0f8f1: ja 0x7fffb2b0fdb0 0x7fffb2b0f8f7: lea -0x70(%rbp), %rsp 0x7fffb2b0f8fb: test $0xf, %spl 0x7fffb2b0f8ff: jz 0x7fffb2b0f906 0x7fffb2b0f905: int3 0x7fffb2b0f906: mov $0xffff000000000000, %r11 0x7fffb2b0f910: cmp %r11, %r14 0x7fffb2b0f913: jz 0x7fffb2b0f91a 0x7fffb2b0f919: int3 0x7fffb2b0f91a: mov $0xffff000000000002, %r11 0x7fffb2b0f924: cmp %r11, %r15 0x7fffb2b0f927: jz 0x7fffb2b0f92e 0x7fffb2b0f92d: int3 0x7fffb2b0f92e: cmp $0x989680, 0x28(%rbp) 0x7fffb2b0f935: jb 0x7fffb2b0f93c 0x7fffb2b0f93b: int3 [ 0] enter 0x7fffb2b0f93c: mov $0xa, %r11 0x7fffb2b0f946: mov %r11, -0x8(%rbp) 0x7fffb2b0f94a: mov $0xa, %r11 0x7fffb2b0f954: mov %r11, -0x10(%rbp) 0x7fffb2b0f958: mov $0x1, 0x2c(%rbp) 0x7fffb2b0f95f: mov $0x44cd08, %r11 0x7fffb2b0f969: mov %rbp, (%r11) 0x7fffb2b0f96c: mov %rbp, %rdi 0x7fffb2b0f96f: mov $0x54fad8, %rsi 0x7fffb2b0f979: mov $0x7ffff75a6300, %r11 0x7fffb2b0f983: call %r11 0x7fffb2b0f986: mov $0x44e838, %r11 0x7fffb2b0f990: mov (%r11), %r11 0x7fffb2b0f993: test %r11, %r11 0x7fffb2b0f996: jnz 0x7fffb2b0fe83 [ 1] init_lazy_reg loc1 0x7fffb2b0f99c: mov $0x0, %r11 0x7fffb2b0f9a6: mov %r11, -0x10(%rbp) [ 3] init_lazy_reg loc0 0x7fffb2b0f9aa: mov $0x0, %r11 0x7fffb2b0f9b4: mov %r11, -0x8(%rbp) [ 5] touch_entry 0x7fffb2b0f9b8: mov $0x6, 0x2c(%rbp) 0x7fffb2b0f9bf: mov $0x44cd08, %r11 0x7fffb2b0f9c9: mov %rbp, (%r11) 0x7fffb2b0f9cc: mov %rbp, %rdi 0x7fffb2b0f9cf: mov $0x54fb00, %rsi 0x7fffb2b0f9d9: mov $0x7ffff75a2310, %r11 0x7fffb2b0f9e3: call %r11 0x7fffb2b0f9e6: mov $0x44e838, %r11 0x7fffb2b0f9f0: mov (%r11), %r11 0x7fffb2b0f9f3: test %r11, %r11 0x7fffb2b0f9f6: jnz 0x7fffb2b0fe83 [ 6] resolve_scope loc2, f(@id0), 1<ThrowIfNotFound|GlobalVar>, 0 0x7fffb2b0f9fc: mov $0x7ffff7f3f970, %rax 0x7fffb2b0fa06: mov %rax, -0x18(%rbp) [ 12] get_from_scope loc3, loc2, f(@id0), 1<ThrowIfNotFound|GlobalVar>, <structure>, 4695632 0x7fffb2b0fa0a: mov 0x47a650, %rax 0x7fffb2b0fa14: mov %rax, -0x20(%rbp) 0x7fffb2b0fa18: mov %rax, 0x5001e0 [ 20] get_by_id loc4, loc3, apply(@id1) predicting None 0x7fffb2b0fa22: mov -0x20(%rbp), %rax 0x7fffb2b0fa26: test %rax, %r15 0x7fffb2b0fa29: jnz 0x7fffb2b0fcd4 0x7fffb2b0fa2f: mov $0xd1e7beef, %r11 0x7fffb2b0fa39: cmp %r11, (%rax) 0x7fffb2b0fa3c: jnz 0x7fffb2b0fcd4 0x7fffb2b0fa42: mov 0x8(%rax), %rax 0x7fffb2b0fa46: mov 0x0(%rax), %rax 0x7fffb2b0fa4a: mov %rax, 0x500200 0x7fffb2b0fa54: mov %rax, -0x28(%rbp) [ 29] jneq_ptr loc4, 1 (0x7ffff7ecff70), 17(->46) 0x7fffb2b0fa58: mov -0x28(%rbp), %rax 0x7fffb2b0fa5c: mov $0x7ffff7ecff70, %r11 0x7fffb2b0fa66: cmp %r11, %rax 0x7fffb2b0fa69: jnz 0x7fffb2b0fbe1 [ 33] mov loc5, loc3 0x7fffb2b0fa6f: mov -0x20(%rbp), %rax 0x7fffb2b0fa73: mov %rax, -0x30(%rbp) [ 36] call_varargs loc4, loc5, Null(@k0), loc1, -7 predicting None 0x7fffb2b0fa77: mov -0x10(%rbp), %rax 0x7fffb2b0fa7b: test %rax, %rax 0x7fffb2b0fa7e: jnz 0x7fffb2b0faf3 0x7fffb2b0fa84: mov 0x28(%rbp), %eax 0x7fffb2b0fa87: cmp $0x10001, %eax 0x7fffb2b0fa8d: ja 0x7fffb2b0faf3 0x7fffb2b0fa93: mov %rax, %rdx 0x7fffb2b0fa96: add $0xd, %rdx 0x7fffb2b0fa9a: inc %rdx 0x7fffb2b0fa9d: and $0xfffffffffffffffe, %rdx 0x7fffb2b0faa1: neg %rdx 0x7fffb2b0faa4: shl $0x3, %rdx 0x7fffb2b0faa8: add %rbp, %rdx 0x7fffb2b0faab: mov $0x44e828, %r11 0x7fffb2b0fab5: cmp %rdx, (%r11) 0x7fffb2b0fab8: ja 0x7fffb2b0faf3 ========> 0x7fffb2b0fabe: mov %eax, 0x28(%rdx) 0x7fffb2b0fac1: mov $0x2, %rcx 0x7fffb2b0facb: mov %rcx, 0x30(%rdx) 0x7fffb2b0facf: movsxd %eax, %rax 0x7fffb2b0fad2: dec %rax 0x7fffb2b0fad5: jz 0x7fffb2b0fb8a 0x7fffb2b0fadb: mov 0x30(%rbp,%rax,8), %rcx 0x7fffb2b0fae0: mov %rcx, 0x30(%rdx,%rax,8) 0x7fffb2b0fae5: dec %rax 0x7fffb2b0fae8: jnz 0x7fffb2b0fadb 0x7fffb2b0faee: jmp 0x7fffb2b0fb8a 0x7fffb2b0faf3: mov -0x10(%rbp), %rdx 0x7fffb2b0faf7: mov %rdx, %rsi 0x7fffb2b0fafa: mov $0xfffffff9, %edx 0x7fffb2b0faff: mov %rbp, %rdi 0x7fffb2b0fb02: mov $0x25, 0x2c(%rbp) 0x7fffb2b0fb09: mov $0x44cd08, %r11 0x7fffb2b0fb13: mov %rbp, (%r11) 0x7fffb2b0fb16: mov $0x7ffff747f050, %r11 0x7fffb2b0fb20: call %r11 0x7fffb2b0fb23: mov $0x44e838, %r11 0x7fffb2b0fb2d: mov (%r11), %r11 0x7fffb2b0fb30: test %r11, %r11 0x7fffb2b0fb33: jnz 0x7fffb2b0fe83 0x7fffb2b0fb39: mov %rax, %rsp 0x7fffb2b0fb3c: mov $0x2, %rdx 0x7fffb2b0fb46: mov -0x10(%rbp), %rcx 0x7fffb2b0fb4a: mov %rax, %rsi 0x7fffb2b0fb4d: mov %rbp, %rdi 0x7fffb2b0fb50: mov $0x25, 0x2c(%rbp) 0x7fffb2b0fb57: mov $0x44cd08, %r11 0x7fffb2b0fb61: mov %rbp, (%r11) 0x7fffb2b0fb64: mov $0x7ffff747f0d0, %r11 0x7fffb2b0fb6e: call %r11 0x7fffb2b0fb71: mov $0x44e838, %r11 0x7fffb2b0fb7b: mov (%r11), %r11 0x7fffb2b0fb7e: test %r11, %r11 0x7fffb2b0fb81: jnz 0x7fffb2b0fe83 0x7fffb2b0fb87: mov %rax, %rdx 0x7fffb2b0fb8a: lea 0x10(%rdx), %rsp 0x7fffb2b0fb8e: mov $0x24, 0x2c(%rbp) 0x7fffb2b0fb95: mov -0x30(%rbp), %rax 0x7fffb2b0fb99: mov %rax, 0x10(%rsp) 0x7fffb2b0fb9e: mov $0x0, %r11 0x7fffb2b0fba8: cmp %r11, %rax 0x7fffb2b0fbab: jnz 0x7fffb2b0fd46 0x7fffb2b0fbb1: mov 0x20(%rax), %rcx 0x7fffb2b0fbb5: mov %rcx, 0x8(%rsp) 0x7fffb2b0fbba: call 0x7fffb2b0fbbf 0x7fffb2b0fbbf: lea -0x70(%rbp), %rsp 0x7fffb2b0fbc3: test $0xf, %spl 0x7fffb2b0fbc7: jz 0x7fffb2b0fbce 0x7fffb2b0fbcd: int3 0x7fffb2b0fbce: mov %rax, 0x500220 0x7fffb2b0fbd8: mov %rax, -0x28(%rbp) [ 44] jmp 21(->65) 0x7fffb2b0fbdc: jmp 0x7fffb2b0fcc0 [ 46] mov loc7, loc3 0x7fffb2b0fbe1: mov -0x20(%rbp), %rax 0x7fffb2b0fbe5: mov %rax, -0x40(%rbp) [ 49] mov loc6, Null(@k0) 0x7fffb2b0fbe9: mov $0x2, %rax 0x7fffb2b0fbf3: mov %rax, -0x38(%rbp) [ 52] create_arguments loc1 0x7fffb2b0fbf7: cmp $0x0, -0x10(%rbp) 0x7fffb2b0fbfc: jnz 0x7fffb2b0fc44 0x7fffb2b0fc02: mov %rbp, %rdi 0x7fffb2b0fc05: mov $0x35, 0x2c(%rbp) 0x7fffb2b0fc0c: mov $0x44cd08, %r11 0x7fffb2b0fc16: mov %rbp, (%r11) 0x7fffb2b0fc19: mov $0x7ffff747e140, %r11 0x7fffb2b0fc23: call %r11 0x7fffb2b0fc26: mov $0x44e838, %r11 0x7fffb2b0fc30: mov (%r11), %r11 0x7fffb2b0fc33: test %r11, %r11 0x7fffb2b0fc36: jnz 0x7fffb2b0fe83 0x7fffb2b0fc3c: mov %rax, -0x10(%rbp) 0x7fffb2b0fc40: mov %rax, -0x8(%rbp) [ 54] mov loc5, loc1 0x7fffb2b0fc44: mov -0x10(%rbp), %rax 0x7fffb2b0fc48: mov %rax, -0x30(%rbp) [ 57] call loc4, loc4, 3, 14 status(Not Set) Original; predicting None 0x7fffb2b0fc4c: mov -0x40(%rbp), %rax 0x7fffb2b0fc50: test %rax, %r15 0x7fffb2b0fc53: jnz 0x7fffb2b0fc66 0x7fffb2b0fc59: mov (%rax), %rax 0x7fffb2b0fc5c: mov %rax, 0x500f98 0x7fffb2b0fc66: lea -0x60(%rbp), %rsp 0x7fffb2b0fc6a: mov $0x3, 0x18(%rsp) 0x7fffb2b0fc72: mov $0x39, 0x2c(%rbp) 0x7fffb2b0fc79: mov -0x28(%rbp), %rax 0x7fffb2b0fc7d: mov %rax, 0x10(%rsp) 0x7fffb2b0fc82: mov $0x0, %r11 0x7fffb2b0fc8c: cmp %r11, %rax 0x7fffb2b0fc8f: jnz 0x7fffb2b0fd7b 0x7fffb2b0fc95: mov 0x20(%rax), %rcx 0x7fffb2b0fc99: mov %rcx, 0x8(%rsp) 0x7fffb2b0fc9e: call 0x7fffb2b0fca3 0x7fffb2b0fca3: lea -0x70(%rbp), %rsp 0x7fffb2b0fca7: test $0xf, %spl 0x7fffb2b0fcab: jz 0x7fffb2b0fcb2 0x7fffb2b0fcb1: int3 0x7fffb2b0fcb2: mov %rax, 0x500240 0x7fffb2b0fcbc: mov %rax, -0x28(%rbp) [ 65] ret loc4 0x7fffb2b0fcc0: mov -0x28(%rbp), %rax 0x7fffb2b0fcc4: test $0xf, %spl 0x7fffb2b0fcc8: jz 0x7fffb2b0fccf 0x7fffb2b0fcce: int3 0x7fffb2b0fccf: mov %rbp, %rsp 0x7fffb2b0fcd2: pop %rbp 0x7fffb2b0fcd3: ret (End Of Main Path) (S) [ 20] get_by_id loc4, loc3, apply(@id1) predicting None 0x7fffb2b0fcd4: mov %rax, %rdx 0x7fffb2b0fcd7: mov $0x500380, %rsi 0x7fffb2b0fce1: mov $0x4533b0, %rcx 0x7fffb2b0fceb: mov %rbp, %rdi 0x7fffb2b0fcee: mov $0x15, 0x2c(%rbp) 0x7fffb2b0fcf5: mov $0x44cd08, %r11 0x7fffb2b0fcff: mov %rbp, (%r11) 0x7fffb2b0fd02: mov $0x7ffff74793e0, %r11 0x7fffb2b0fd0c: call %r11 0x7fffb2b0fd0f: mov $0x44e838, %r11 0x7fffb2b0fd19: mov (%r11), %r11 0x7fffb2b0fd1c: test %r11, %r11 0x7fffb2b0fd1f: jnz 0x7fffb2b0fe83 0x7fffb2b0fd25: mov %rax, 0x500200 0x7fffb2b0fd2f: mov %rax, -0x28(%rbp) 0x7fffb2b0fd33: mov $0x500514, %r11 0x7fffb2b0fd3d: add $0x1, (%r11) 0x7fffb2b0fd41: jmp 0x7fffb2b0fa58 (S) [ 36] call_varargs loc4, loc5, Null(@k0), loc1, -7 predicting None 0x7fffb2b0fd46: call 0x7fffb2af3960 0x7fffb2b0fd4b: lea -0x70(%rbp), %rsp 0x7fffb2b0fd4f: test $0xf, %spl 0x7fffb2b0fd53: jz 0x7fffb2b0fd5a 0x7fffb2b0fd59: int3 0x7fffb2b0fd5a: mov %rax, 0x500220 0x7fffb2b0fd64: mov %rax, -0x28(%rbp) 0x7fffb2b0fd68: mov $0x50051c, %r11 0x7fffb2b0fd72: add $0x1, (%r11) 0x7fffb2b0fd76: jmp 0x7fffb2b0fbdc (S) [ 57] call loc4, loc4, 3, 14 status(Not Set) Original; predicting None 0x7fffb2b0fd7b: call 0x7fffb2af3960 0x7fffb2b0fd80: lea -0x70(%rbp), %rsp 0x7fffb2b0fd84: test $0xf, %spl 0x7fffb2b0fd88: jz 0x7fffb2b0fd8f 0x7fffb2b0fd8e: int3 0x7fffb2b0fd8f: mov %rax, 0x500240 0x7fffb2b0fd99: mov %rax, -0x28(%rbp) 0x7fffb2b0fd9d: mov $0x500524, %r11 0x7fffb2b0fda7: add $0x1, (%r11) 0x7fffb2b0fdab: jmp 0x7fffb2b0fcc0 (End Of Slow Path) 0x7fffb2b0fdb0: mov $0x54e2a0, %rsi 0x7fffb2b0fdba: mov %rbp, %rdi 0x7fffb2b0fdbd: mov $0x1, 0x2c(%rbp) 0x7fffb2b0fdc4: mov $0x44cd08, %r11 0x7fffb2b0fdce: mov %rbp, (%r11) 0x7fffb2b0fdd1: mov $0x7ffff7479000, %r11 0x7fffb2b0fddb: call %r11 0x7fffb2b0fdde: mov $0x44e838, %r11 0x7fffb2b0fde8: mov (%r11), %r11 0x7fffb2b0fdeb: test %r11, %r11 0x7fffb2b0fdee: jnz 0x7fffb2b0fe7a 0x7fffb2b0fdf4: mov $0x54e2e2, %r11 0x7fffb2b0fdfe: mov $0x0, (%r11) 0x7fffb2b0fe02: push %rbp 0x7fffb2b0fe03: mov %rsp, %rbp 0x7fffb2b0fe06: mov $0x54e2a0, %r11 0x7fffb2b0fe10: mov %r11, 0x10(%rbp) 0x7fffb2b0fe14: mov 0x28(%rbp), %edx 0x7fffb2b0fe17: cmp $0x1, %edx 0x7fffb2b0fe1a: jae 0x7fffb2b0f8d2 0x7fffb2b0fe20: mov %rbp, %rdi 0x7fffb2b0fe23: mov $0x1, 0x2c(%rbp) 0x7fffb2b0fe2a: mov $0x44cd08, %r11 0x7fffb2b0fe34: mov %rbp, (%r11) 0x7fffb2b0fe37: mov $0x7ffff74790a0, %r11 0x7fffb2b0fe41: call %r11 0x7fffb2b0fe44: mov $0x44e838, %r11 0x7fffb2b0fe4e: mov (%r11), %r11 0x7fffb2b0fe51: test %r11, %r11 0x7fffb2b0fe54: jnz 0x7fffb2b0fe7a 0x7fffb2b0fe5a: test %eax, %eax 0x7fffb2b0fe5c: jz 0x7fffb2b0f8d2 0x7fffb2b0fe62: mov $0x472600, %rsi 0x7fffb2b0fe6c: mov (%rsi,%rax,8), %rsi 0x7fffb2b0fe70: call 0x7fffb2afd2a0 0x7fffb2b0fe75: jmp 0x7fffb2b0f8d2 0x7fffb2b0fe7a: mov 0x0(%rbp), %rsi 0x7fffb2b0fe7e: jmp 0x7fffb2b0fe86 0x7fffb2b0fe83: mov %rbp, %rsi 0x7fffb2b0fe86: mov $0x442960, %rdi 0x7fffb2b0fe90: mov $0x7ffff747fe10, %r11 0x7fffb2b0fe9a: call %r11 0x7fffb2b0fe9d: mov $0x44e750, %rdx 0x7fffb2b0fea7: mov (%rdx), %rdx 0x7fffb2b0feaa: jmp %rdx
Csaba Osztrogonác
Comment 4 2014-02-25 08:35:12 PST
Could you possibly give use any hint what can be the problem?
Geoffrey Garen
Comment 5 2014-02-25 09:16:31 PST
It's clear that something has gone wrong in the part of call_varargs that's responsible for copying a variable list of arguments onto the stack. Given that this test uses "many args", it's possible that we've overflowed the stack, but our stack check has somehow failed: stack check: 0x7fffb2b0faa8: add %rbp, %rdx 0x7fffb2b0faab: mov $0x44e828, %r11 0x7fffb2b0fab5: cmp %rdx, (%r11) 0x7fffb2b0fab8: ja 0x7fffb2b0faf3 crash: 0x7fffb2b0fabe: mov %eax, 0x28(%rdx) %rdx is near %rbp and %rsp, but substantially higher: rdx 0x7fffffef9fe0 140737487282144 rbp 0x7ffffff7a050 0x7ffffff7a050 rsp 0x7ffffff79fe0 0x7ffffff79fe0 It does seem like we're checking the wrong thing here.
Geoffrey Garen
Comment 6 2014-02-25 09:20:16 PST
> %rdx is near %rbp and %rsp, but substantially higher: *lower*
Geoffrey Garen
Comment 7 2014-02-25 09:21:32 PST
In your debugging session, can you print out the stack limit values in the VM, and compare them to the value of %rdx, and also compare them to the value of the OS's view of the end of the stack region in memory?
Michael Saboff
Comment 8 2014-02-25 09:24:32 PST
You didn't say what kind of fault you're taking, but I'll assume some kind of set or bus fault. A quick look at the source (JITCall.cpp::compileLoadVarargs) and disassembly and I'd say we are in the "if (canOptimize)" case since we are using arguments. I've mixed the source and assembly together. The faulting line is when we set the callee's arg count from the caller's arg count. Before that, we have made space on the stack for the callee frame (in rdx / regT1) by subtracting the size of the new frame from bp. The difference between the caller frame pointer (rbp) and callee frame pointer (rdx) is slightly more than .5MB. I wonder if the stack limit check is working as intended. If it isn't, you could be exceeding the stack. For example on Mac, we expect that this particular test, g.apply(null, new Array(65537)) to throw a stack size exceeded exception. I'll also agree that function-apply-many-args.js needs work. The array sizes that are elected to work and those that we expect to exceed the stake are now dependent on the native stack size of each platform and not on the stack size we allocate. Saying that, JSC shouldn't crash on but throw the exception. Look at what is in VM::m_jsStackLimit (0x44e828 for this code block, but different for each VM). Looking at this, I see a minor bug, but I don't think it affects you. We are using m_jsStackLimit and not m_stackLimit for the stack check. This is fine if you use the standard LLInt, but is broken if you use the LLInt C Loop. I'll file another bug and take care of that. [ 36] call_varargs loc4, loc5, Null(@k0), loc1, -7 predicting None if (canOptimize) { emitGetVirtualRegister(arguments, regT0); 0x7fffb2b0fa77: mov -0x10(%rbp), %rax slowCase.append(branch64(NotEqual, regT0, TrustedImm64(JSValue::encode(JSValue())))); 0x7fffb2b0fa7b: test %rax, %rax 0x7fffb2b0fa7e: jnz 0x7fffb2b0faf3 emitGetFromCallFrameHeader32(JSStack::ArgumentCount, regT0); 0x7fffb2b0fa84: mov 0x28(%rbp), %eax slowCase.append(branch32(Above, regT0, TrustedImm32(Arguments::MaxArguments + 1))); 0x7fffb2b0fa87: cmp $0x10001, %eax 0x7fffb2b0fa8d: ja 0x7fffb2b0faf3 // regT0: argumentCountIncludingThis move(regT0, regT1); 0x7fffb2b0fa93: mov %rax, %rdx add64(TrustedImm32(-firstFreeRegister + JSStack::CallFrameHeaderSize), regT1); 0x7fffb2b0fa96: add $0xd, %rdx // regT1 now has the required frame size in Register units // Round regT1 to next multiple of stackAlignmentRegisters() add64(TrustedImm32(stackAlignmentRegisters() - 1), regT1); 0x7fffb2b0fa9a: inc %rdx and64(TrustedImm32(~(stackAlignmentRegisters() - 1)), regT1); 0x7fffb2b0fa9d: and $0xfffffffffffffffe, %rdx neg64(regT1); 0x7fffb2b0faa1: neg %rdx lshift64(TrustedImm32(3), regT1); 0x7fffb2b0faa4: shl $0x3, %rdx addPtr(callFrameRegister, regT1); 0x7fffb2b0faa8: add %rbp, %rdx // regT1: newCallFrame slowCase.append(branchPtr(Above, AbsoluteAddress(m_vm->addressOfJSStackLimit()), regT1)); 0x7fffb2b0faab: mov $0x44e828, %r11 0x7fffb2b0fab5: cmp %rdx, (%r11) 0x7fffb2b0fab8: ja 0x7fffb2b0faf3 // Initialize ArgumentCount. store32(regT0, Address(regT1, JSStack::ArgumentCount * static_cast<int>(sizeof(Register)) + OBJECT_OFFSETOF(EncodedValueDescriptor, asBits.payload))); ========> 0x7fffb2b0fabe: mov %eax, 0x28(%rdx) // Initialize 'this'. emitGetVirtualRegister(thisValue, regT2); 0x7fffb2b0fac1: mov $0x2, %rcx store64(regT2, Address(regT1, CallFrame::thisArgumentOffset() * static_cast<int>(sizeof(Register)))); 0x7fffb2b0facb: mov %rcx, 0x30(%rdx) ...
Michael Saboff
Comment 9 2014-02-25 09:40:41 PST
Filed https://bugs.webkit.org/show_bug.cgi?id=129314 to cover the issue using m_jsStackLimit and not m_stackLimit for the stack check for the baseline and DFG JITs.
Mark Lam
Comment 10 2014-02-25 10:16:53 PST
(In reply to comment #8) > Look at what is in VM::m_jsStackLimit (0x44e828 for this code block, but different for each VM). Looking at this, I see a minor bug, but I don't think it affects you. We are using m_jsStackLimit and not m_stackLimit for the stack check. This is fine if you use the standard LLInt, but is broken if you use the LLInt C Loop. I'll file another bug and take care of that. What is the issue? On JIT builds, m_jsStackLimit and m_stackLimit are identical because they are in a union (by design). On C loop LLINT, they are distinct fields in a struct by design because the JS stack is not on the C stack, and JIT code is not executed.
Mark Lam
Comment 11 2014-02-25 11:01:40 PST
Ossy, can you provide the svn revision # that you’re seeing this on? Also, is this on 32-bit or 64-bit? I see 2 jsc stress test failures on 32-bit x86 on Mac, but not the same one as the ones in this bug.
Mark Lam
Comment 12 2014-02-25 11:33:05 PST
(In reply to comment #11) > I see 2 jsc stress test failures on 32-bit x86 on Mac, but not the same one as the ones in this bug. FYI, ref: https://bugs.webkit.org/show_bug.cgi?id=129318
Zan Dobersek
Comment 13 2014-02-25 15:54:54 PST
WTF::StackBounds::initialize() sets up: - origin at 0x7ffffffff000, - bound at 0x7fffff7ff000 for 8MB in size. APIEntryShim in jsc.cpp[1], through JSLock, raises the bound to 0x7fffffc1df20 by calculating the recursion limit for the user stack that starts at 0x7fffffffdc30, is 4MB in size and with the reserved zone 128kB in size. The bound is the address to which m_stackLimit points at the time of crash, with m_stackLimit address stored in the %r11 register. [1] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/jsc.cpp#L1076
Geoffrey Garen
Comment 14 2014-02-25 16:02:19 PST
> WTF::StackBounds::initialize() sets up: > - origin at 0x7ffffffff000, > - bound at 0x7fffff7ff000 > for 8MB in size. 0x7fffffef9fe0 is well above 0x7fffff7ff000, so we have not overflowed the value set by WTF::StackBounds. But is that value actually correct? Can you double-check with some system tool to see what the actual VM mapping for the main thread's stack is?
Zan Dobersek
Comment 15 2014-02-25 16:39:41 PST
Created attachment 225196 [details] Debugging data - info proc all, instructions, registers with ToT This text file contains GDB output of the `info proc all` command. I hope that properly represents the requested memory mappings. There's also the call_varargs instruction list and register values I produced with a ToT build.
Michael Saboff
Comment 16 2014-02-25 16:48:06 PST
(In reply to comment #15) > Created an attachment (id=225196) [details] > Debugging data - info proc all, instructions, registers with ToT > > This text file contains GDB output of the `info proc all` command. I hope that properly represents the requested memory mappings. > > There's also the call_varargs instruction list and register values I produced with a ToT build. In the attachment, the memory map shows: Start Addr End Addr Size Offset objfile 0x7ffffff7b000 0x7ffffffff000 0x84000 0x0 [stack] And the registers show: Registers: rdx 0x7fffffefc380 140737487291264 rbp 0x7ffffff7c400 0x7ffffff7c400 rsp 0x7ffffff7c380 0x7ffffff7c380 rdx - 0x7fffffefc380 - Isn't part of the mapped address space. The current stack allocation isn't big enough for this call. The value pointed to by VM::m_jsStackLimit is also outside of the allocated stack area. %r11: 0x4537c0: 0x00007fffffc1df20 Could it be that when VM::m_jsStackLimit was updated, the stack allocation wasn't increased? Or maybe VM:m_jsStack shouldn't have been updated.
Zan Dobersek
Comment 17 2014-02-27 01:09:06 PST
When running the test case with LLInt enabled the stack does get allocated down to 0x7fffffefc000 for a size just over a megabyte, so I reckon that might be the issue.
Zan Dobersek
Comment 18 2014-03-02 08:28:35 PST
Disabling the canOptimize branch at build-time in JIT::compileLoadVarargs() avoids the problem, but is obviously not the solution. I believe the generated JIT code for 'function g() { return f.apply(null, arguments); }' is missing the create_arguments opcode in the optimized path. Modifying the source to 'function g() { arguments; return f.apply(null, arguments); }' (as is the case with the h() function in the function-apply-many-args.js file) includes that opcode through BytecodeGenerator::local()[1] (via BytecodeGenerator::createArgumentsIfNecessary()) [2]. Calling BytecodeGenerator::createArgumentsIfNecessary() from BytecodeGenerator::emitCallVarargs()[3] fixes this, generating create_arguments which enables the canOptimize branch to function properly, and removing the crashes. Does that make sense? [1] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1168 [2] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1613 [3] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1789
Oliver Hunt
Comment 19 2014-03-03 10:55:05 PST
(In reply to comment #18) > Disabling the canOptimize branch at build-time in JIT::compileLoadVarargs() avoids the problem, but is obviously not the solution. > > I believe the generated JIT code for 'function g() { return f.apply(null, arguments); }' is missing the create_arguments opcode in the optimized path. Modifying the source to 'function g() { arguments; return f.apply(null, arguments); }' (as is the case with the h() function in the function-apply-many-args.js file) includes that opcode through BytecodeGenerator::local()[1] (via BytecodeGenerator::createArgumentsIfNecessary()) [2]. > > Calling BytecodeGenerator::createArgumentsIfNecessary() from BytecodeGenerator::emitCallVarargs()[3] fixes this, generating create_arguments which enables the canOptimize branch to function properly, and removing the crashes. Does that make sense? > > [1] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1168 > [2] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1613 > [3] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1789 f.apply(thing, arguments) should not instantiate the arguments object -- it should essentially just be doing a move
Zan Dobersek
Comment 20 2014-03-03 12:43:16 PST
(In reply to comment #19) > (In reply to comment #18) > > Disabling the canOptimize branch at build-time in JIT::compileLoadVarargs() avoids the problem, but is obviously not the solution. > > > > I believe the generated JIT code for 'function g() { return f.apply(null, arguments); }' is missing the create_arguments opcode in the optimized path. Modifying the source to 'function g() { arguments; return f.apply(null, arguments); }' (as is the case with the h() function in the function-apply-many-args.js file) includes that opcode through BytecodeGenerator::local()[1] (via BytecodeGenerator::createArgumentsIfNecessary()) [2]. > > > > Calling BytecodeGenerator::createArgumentsIfNecessary() from BytecodeGenerator::emitCallVarargs()[3] fixes this, generating create_arguments which enables the canOptimize branch to function properly, and removing the crashes. Does that make sense? > > > > [1] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1168 > > [2] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1613 > > [3] http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp#L1789 > > f.apply(thing, arguments) should not instantiate the arguments object -- it should essentially just be doing a move But is the canOptimize branch in JIT::compileLoadVarargs() expecting to operate on an existing arguments object or can it optimize without it? A move is obviously ideal, but there's nothing to move.
Zan Dobersek
Comment 21 2014-03-03 12:47:10 PST
(In reply to comment #8) > I wonder if the stack limit check is working as intended. If it isn't, you could be exceeding the stack. For example on Mac, we expect that this particular test, g.apply(null, new Array(65537)) to throw a stack size exceeded exception. > The crashing test case is actually 'g.apply(null, new Array(65536))', so it just passes the MaxArguments + 1 test and is expected to succeed (i.e. not throw an exception).
Geoffrey Garen
Comment 22 2014-03-03 13:35:58 PST
The key question here is, "Why does m_jsStackLimit hold a value that is outside the thread's allocated stack space?"
Carlos Alberto Lopez Perez
Comment 23 2014-04-28 11:59:54 PDT
I have noticed that on the GTK ARM buildbot the number of failures are around 180: http://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/1678/steps/jscore-test The EFL ARM buildbots are not executing the build step jscore-test, so I don't know if this is something specific of the GTK ARM buildbot, or is something more general related with the ARM platform. Can any of you try the run-jsc-stress-tests tests on an ARM board?
Csaba Osztrogonác
Comment 24 2014-04-29 05:15:49 PDT
(In reply to comment #23) > I have noticed that on the GTK ARM buildbot the number of failures are around 180: > > http://build.webkit.org/builders/GTK%20Linux%20ARM%20Release/builds/1678/steps/jscore-test > > The EFL ARM buildbots are not executing the build step jscore-test, so I don't know if this is something specific of the GTK ARM buildbot, or is something more general related with the ARM platform. > > > Can any of you try the run-jsc-stress-tests tests on an ARM board? No it isn't GTK specific bug, we have similar failures on EFL too, check the EFL Thumb2 tester bot: http://build.webkit.sed.hu/builders/EFL%20ARMv7%20Linux%20Release%20%28Build%29/builds/3882 This is a regression caused by merging the CStack branch, we started investigating it, but can't find the root of the problem yet. I'll file a bug report today and summarize our results.
Csaba Osztrogonác
Comment 25 2014-05-09 06:18:07 PDT
new bug report for the ARM Thumb2 __proto__ bug: https://bugs.webkit.org/show_bug.cgi?id=132740
Zan Dobersek
Comment 26 2014-05-19 10:00:45 PDT
Created attachment 231696 [details] Patch
Geoffrey Garen
Comment 27 2014-05-19 11:34:48 PDT
Comment on attachment 231696 [details] Patch Can you write this to share its implementation with the Windows version? Both platforms have the same problem, and it's not so great that they currently use slightly different solutions.
Carlos Alberto Lopez Perez
Comment 28 2014-05-21 11:10:00 PDT
If someone wonders why the JSC tests are now green on the GTK or EFL bots please look at: https://bugs.webkit.org/show_bug.cgi?id=133149
Zan Dobersek
Comment 29 2014-06-22 11:47:03 PDT
(In reply to comment #27) > (From update of attachment 231696 [details]) > Can you write this to share its implementation with the Windows version? Both platforms have the same problem, and it's not so great that they currently use slightly different solutions. Unfortunately I don't have access to a Windows system to test this out. The main issue I have with the workaround that landed in bug #129429 is that it's run every time the VM instance has its stack limit updated. Linux ports can do this only once per thread -- not sure about Windows. The Linux-specific solution also has a quite delicate implementation as it has to work under both Clang and GCC and it might not be suitable for MSVC. Also CC-ing peavo who implemented the workaround for Windows and might be able to test out this approach. Anyway, I'd like for this to finally land and that a single solution is refined afterwards, if possible.
Michael Saboff
Comment 30 2014-06-23 09:49:33 PDT
Comment on attachment 231696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231696&action=review Pretty close. Just a few items. > Source/WTF/wtf/StackBounds.cpp:103 > +// In the case of really big stacks we only initialize first 256MB of it. Such sizes are achievable if the stack 256MB seems huge. Shouldn't this be Options::maxPerThreadStackUsage? > Source/WTF/wtf/StackBounds.cpp:112 > +#if COMPILER(GCC) && !COMPILER(CLANG) > +#define UNOPTIMIZED __attribute__((optimize("O0"))) > +#else > +#define UNOPTIMIZED > +#endif Please add a FIXME here, referencing a new webkit bug and possibly a gcc bug number? > Source/WTF/wtf/StackBounds.cpp:116 > +// builds by using the GCC-specific attribute. Passing a reference into this function prevents both GCC and Clang to > +// optimize away the first initializeStack() call in StackBounds::initialize(). Change "to optimize" to "from optimizing" > Source/WTF/wtf/StackBounds.cpp:123 > + stackInitialized = true; Why can't this just be a return? If it is a return, then I don't think we need "stackInitialized" except maybe to prevent the compiler optimizing away issue noted above.
Mark Lam
Comment 31 2014-06-23 10:00:47 PDT
Comment on attachment 231696 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231696&action=review >> Source/WTF/wtf/StackBounds.cpp:103 >> +// In the case of really big stacks we only initialize first 256MB of it. Such sizes are achievable if the stack > > 256MB seems huge. Shouldn't this be Options::maxPerThreadStackUsage? It would be a layer violation for WTF to use JSC::Options::maxPerThreadStackUsage.
Mark Lam
Comment 32 2014-06-23 10:04:59 PDT
(In reply to comment #29) > (In reply to comment #27) > > (From update of attachment 231696 [details] [details]) > > Can you write this to share its implementation with the Windows version? Both platforms have the same problem, and it's not so great that they currently use slightly different solutions. > > Unfortunately I don't have access to a Windows system to test this out. The main issue I have with the workaround that landed in bug #129429 is that it's run every time the VM instance has its stack limit updated. Linux ports can do this only once per thread -- not sure about Windows. The Linux-specific solution also has a quite delicate implementation as it has to work under both Clang and GCC and it might not be suitable for MSVC. Can you please clarify what you mean by "Linux ports can do this only once per thread”? Are you referring to some pthread limitation or something else? Thanks.
Michael Saboff
Comment 33 2014-06-23 10:34:09 PDT
(In reply to comment #31) > (From update of attachment 231696 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=231696&action=review > > >> Source/WTF/wtf/StackBounds.cpp:103 > >> +// In the case of really big stacks we only initialize first 256MB of it. Such sizes are achievable if the stack > > > > 256MB seems huge. Shouldn't this be Options::maxPerThreadStackUsage? > > It would be a layer violation for WTF to use JSC::Options::maxPerThreadStackUsage. Things could be restructured to pass a stack size down to StackBounds::initialize() or by providing a new API that JSC calls to do this initialization as part of initializing the VM. Given the huge difference between the current values of Options::maxPerThreadStackUsage (4MB) and the proposed stack initialization limit of 256MB proposed with this patch, using a common value makes sense.
Csaba Osztrogonác
Comment 34 2014-07-14 04:55:47 PDT
Just a note: This failure is appeared on the GTK/EFL bots, because of modifying the test by https://bugs.webkit.org/show_bug.cgi?id=126588
Csaba Osztrogonác
Comment 35 2014-07-14 04:56:12 PDT
(In reply to comment #34) > Just a note: This failure is appeared on the GTK/EFL bots, because of > modifying the test by https://bugs.webkit.org/show_bug.cgi?id=126588 But the bug is still valid, of coursse.
Zan Dobersek
Comment 36 2016-09-05 00:22:27 PDT
This was resolved some time ago.
Note You need to log in before you can comment on or make changes to this bug.