Ignore:
Timestamp:
Jul 25, 2017, 1:56:04 PM (8 years ago)
Author:
[email protected]
Message:

Fix bugs in probe code to change sp on x86, x86_64 and 32-bit ARM.
​https://bugs.webkit.org/show_bug.cgi?id=174809
<rdar://problem/33504759>

Reviewed by Filip Pizlo.

  1. When the probe handler function changes the sp register to point to the region of stack in the middle of the ProbeContext on the stack, there is a bug where the ProbeContext's register values to be restored can be over-written before they can be restored. This is now fixed.
  1. Added more robust probe tests for changing the sp register.
  1. Made existing probe tests to ensure that probe handlers were actually called.
  1. Added some verification to testProbePreservesGPRS().
  1. Change all the probe tests to fail early on discovering an error instead of batching till the end of the test. This helps point a finger to the failing issue earlier.

This patch was tested on x86, x86_64, and ARMv7. ARM64 probe code will be fixed
next in ​https://bugs.webkit.org/show_bug.cgi?id=174697.

  • assembler/MacroAssemblerARM.cpp:
  • assembler/MacroAssemblerARMv7.cpp:
  • assembler/MacroAssemblerX86Common.cpp:
  • assembler/testmasm.cpp:

(JSC::testProbeReadsArgumentRegisters):
(JSC::testProbeWritesArgumentRegisters):
(JSC::testProbePreservesGPRS):
(JSC::testProbeModifiesStackPointer):
(JSC::testProbeModifiesStackPointerToInsideProbeContextOnStack):
(JSC::testProbeModifiesStackPointerToNBytesBelowSP):
(JSC::testProbeModifiesProgramCounter):
(JSC::run):

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.cpp

    r219740 r219885  
    172172    //     esp[0 * ptrSize]: eflags
    173173    //     esp[1 * ptrSize]: return address / saved eip
    174     //     esp[2 * ptrSize]: probeFunction
    175     //     esp[3 * ptrSize]: arg1
     174    //     esp[2 * ptrSize]: probe handler function
     175    //     esp[3 * ptrSize]: probe arg
    176176    //     esp[4 * ptrSize]: saved eax
    177177    //     esp[5 * ptrSize]: saved esp
    … …  
    241241    // There are 6 more registers left to restore:
    242242    //     eax, ecx, ebp, esp, eip, and eflags.
    243     // We need to handle these last few restores carefully because:
    244     //
    245     // 1. We need to push the return address on the stack for ret to use.
    246     //    That means we need to write to the stack.
    247     // 2. The user probe function may have altered the restore value of esp to
    248     //    point to the vicinity of one of the restore values for the remaining
    249     //    registers left to be restored.
    250     //    That means, for requirement 1, we may end up writing over some of the
    251     //    restore values. We can check for this, and first copy the restore
    252     //    values to a "safe area" on the stack before commencing with the action
    253     //    for requirement 1.
    254     // 3. For requirement 2, we need to ensure that the "safe area" is
    255     //    protected from interrupt handlers overwriting it. Hence, the esp needs
    256     //    to be adjusted to include the "safe area" before we start copying the
    257     //    the restore values.
    258 
     243
     244    // The restoration process at ctiMasmProbeTrampolineEnd below works by popping
     245    // 5 words off the stack into eflags, eax, ecx, ebp, and eip. These 5 words need
     246    // to be pushed on top of the final esp value so that just by popping the 5 words,
     247    // we'll get the esp that the probe wants to set. Let's call this area (for storing
     248    // these 5 words) the restore area.
     249    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%ebp), %ecx" "\n"
     250    "subl $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %ecx" "\n"
     251
     252    // ecx now points to the restore area.
     253
     254    // Before we copy values from the ProbeContext to the restore area, we need to
     255    // make sure that the restore area does not overlap any of the values that we'll
     256    // be copying from in the ProbeContext. All the restore values to be copied from
     257    // comes from offset <= PROBE_CPU_EFLAGS_OFFSET in the ProbeContext.
    259258    "movl %ebp, %eax" "\n"
    260259    "addl $" STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) ", %eax" "\n"
    261     "cmpl %eax, " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%ebp)" "\n"
     260    "cmpl %eax, %ecx" "\n"
    262261    "jg " SYMBOL_STRING(ctiMasmProbeTrampolineEnd) "\n"
    263262
    264     // Locate the "safe area" at 2x sizeof(ProbeContext) below where the new
    265     // rsp will be. This time we don't have to 32-byte align it because we're
    266     // not using this area to store any xmm regs.
    267     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%ebp), %eax" "\n"
     263    // Getting here means that the restore area will overlap the ProbeContext data
     264    // that we will need to get the restoration values from. So, let's move that
     265    // data to a safe place before we start writing into the restore area.
     266    // Let's locate the "safe area" at 2x sizeof(ProbeContext) below where the
     267    // restore area. This ensures that:
     268    // 1. The safe area does not overlap the restore area.
     269    // 2. The safe area does not overlap the ProbeContext.
     270    //    This makes it so that we can use memcpy (does not require memmove) semantics
     271    //    to copy the restore values to the safe area.
     272    // Note: the safe area does not have to 32-byte align it because we're not using
     273    // it to store any xmm regs.
     274    "movl %ecx, %eax" "\n"
    268275    "subl $2 * " STRINGIZE_VALUE_OF(PROBE_ALIGNED_SIZE) ", %eax" "\n"
     276
     277    // eax now points to the safe area.
     278
     279    // Make sure the stack pointer points to the safe area. This ensures that the
     280    // safe area is protected from interrupt handlers overwriting it.
    269281    "movl %eax, %esp" "\n"
    270282
    271     "subl $" STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) ", %eax" "\n"
    272283    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%ebp), %ecx" "\n"
    273284    "movl %ecx, " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%eax)" "\n"
    … …  
    284295    "movl %eax, %ebp" "\n"
    285296
     297    // We used ecx above as scratch register. Let's restore it to points to the
     298    // restore area.
     299    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%ebp), %ecx" "\n"
     300    "subl $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %ecx" "\n"
     301
     302    // ecx now points to the restore area.
     303
    286304    SYMBOL_STRING(ctiMasmProbeTrampolineEnd) ":" "\n"
    287     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%ebp), %eax" "\n"
    288     "subl $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %eax" "\n"
    289     // At this point, %esp should be < %eax.
    290 
    291     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) "(%ebp), %ecx" "\n"
    292     "movl %ecx, 0 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%eax)" "\n"
    293     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%ebp), %ecx" "\n"
    294     "movl %ecx, 1 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%eax)" "\n"
    295     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ECX_OFFSET) "(%ebp), %ecx" "\n"
    296     "movl %ecx, 2 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%eax)" "\n"
    297     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EBP_OFFSET) "(%ebp), %ecx" "\n"
    298     "movl %ecx, 3 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%eax)" "\n"
    299     "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EIP_OFFSET) "(%ebp), %ecx" "\n"
    300     "movl %ecx, 4 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%eax)" "\n"
    301     "movl %eax, %esp" "\n"
    302 
     305
     306    // Copy remaining restore values from the ProbeContext to the restore area.
     307    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) "(%ebp), %eax" "\n"
     308    "movl %eax, 0 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%ecx)" "\n"
     309    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%ebp), %eax" "\n"
     310    "movl %eax, 1 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%ecx)" "\n"
     311    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_ECX_OFFSET) "(%ebp), %eax" "\n"
     312    "movl %eax, 2 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%ecx)" "\n"
     313    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EBP_OFFSET) "(%ebp), %eax" "\n"
     314    "movl %eax, 3 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%ecx)" "\n"
     315    "movl " STRINGIZE_VALUE_OF(PROBE_CPU_EIP_OFFSET) "(%ebp), %eax" "\n"
     316    "movl %eax, 4 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%ecx)" "\n"
     317    "movl %ecx, %esp" "\n"
     318
     319    // Do the remaining restoration by popping off the restore area.
    303320    "popfd" "\n"
    304321    "popl %eax" "\n"
    … …  
    322339    //     esp[0 * ptrSize]: rflags
    323340    //     esp[1 * ptrSize]: return address / saved rip
    324     //     esp[2 * ptrSize]: probeFunction
    325     //     esp[3 * ptrSize]: arg1
     341    //     esp[2 * ptrSize]: probe handler function
     342    //     esp[3 * ptrSize]: probe arg
    326343    //     esp[4 * ptrSize]: saved rax
    327344    //     esp[5 * ptrSize]: saved rsp
    … …  
    421438    // There are 6 more registers left to restore:
    422439    //     rax, rcx, rbp, rsp, rip, and rflags.
    423     // We need to handle these last few restores carefully because:
    424     //
    425     // 1. We need to push the return address on the stack for ret to use
    426     //    That means we need to write to the stack.
    427     // 2. The user probe function may have altered the restore value of esp to
    428     //    point to the vicinity of one of the restore values for the remaining
    429     //    registers left to be restored.
    430     //    That means, for requirement 1, we may end up writing over some of the
    431     //    restore values. We can check for this, and first copy the restore
    432     //    values to a "safe area" on the stack before commencing with the action
    433     //    for requirement 1.
    434     // 3. For both requirement 2, we need to ensure that the "safe area" is
    435     //    protected from interrupt handlers overwriting it. Hence, the esp needs
    436     //    to be adjusted to include the "safe area" before we start copying the
    437     //    the restore values.
    438 
     440
     441    // The restoration process at ctiMasmProbeTrampolineEnd below works by popping
     442    // 5 words off the stack into rflags, rax, rcx, rbp, and rip. These 5 words need
     443    // to be pushed on top of the final esp value so that just by popping the 5 words,
     444    // we'll get the esp that the probe wants to set. Let's call this area (for storing
     445    // these 5 words) the restore area.
     446    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%rbp), %rcx" "\n"
     447    "subq $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %rcx" "\n"
     448
     449    // rcx now points to the restore area.
     450
     451    // Before we copy values from the ProbeContext to the restore area, we need to
     452    // make sure that the restore area does not overlap any of the values that we'll
     453    // be copying from in the ProbeContext. All the restore values to be copied from
     454    // comes from offset <= PROBE_CPU_EFLAGS_OFFSET in the ProbeContext.
    439455    "movq %rbp, %rax" "\n"
    440456    "addq $" STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) ", %rax" "\n"
    441     "cmpq %rax, " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%rbp)" "\n"
     457    "cmpq %rax, %rcx" "\n"
    442458    "jg " SYMBOL_STRING(ctiMasmProbeTrampolineEnd) "\n"
    443459
    444     // Locate the "safe area" at 2x sizeof(ProbeContext) below where the new
    445     // rsp will be. This time we don't have to 32-byte align it because we're
    446     // not using to store any xmm regs.
    447     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%rbp), %rax" "\n"
     460    // Getting here means that the restore area will overlap the ProbeContext data
     461    // that we will need to get the restoration values from. So, let's move that
     462    // data to a safe place before we start writing into the restore area.
     463    // Let's locate the "safe area" at 2x sizeof(ProbeContext) below where the
     464    // restore area. This ensures that:
     465    // 1. The safe area does not overlap the restore area.
     466    // 2. The safe area does not overlap the ProbeContext.
     467    //    This makes it so that we can use memcpy (does not require memmove) semantics
     468    //    to copy the restore values to the safe area.
     469    // Note: the safe area does not have to 32-byte align it because we're not using
     470    // it to store any xmm regs.
     471    "movq %rcx, %rax" "\n"
    448472    "subq $2 * " STRINGIZE_VALUE_OF(PROBE_ALIGNED_SIZE) ", %rax" "\n"
     473
     474    // rax now points to the safe area.
     475
     476    // Make sure the stack pointer points to the safe area. This ensures that the
     477    // safe area is protected from interrupt handlers overwriting it.
    449478    "movq %rax, %rsp" "\n"
    450479
    … …  
    463492    "movq %rax, %rbp" "\n"
    464493
     494    // We used rcx above as scratch register. Let's restore it to points to the
     495    // restore area.
     496    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%rbp), %rcx" "\n"
     497    "subq $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %rcx" "\n"
     498
     499    // rcx now points to the restore area.
     500
    465501    SYMBOL_STRING(ctiMasmProbeTrampolineEnd) ":" "\n"
    466     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ESP_OFFSET) "(%rbp), %rax" "\n"
    467     "subq $5 * " STRINGIZE_VALUE_OF(PTR_SIZE) ", %rax" "\n"
    468     // At this point, %rsp should be < %rax.
    469 
    470     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) "(%rbp), %rcx" "\n"
    471     "movq %rcx, 0 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rax)" "\n"
    472     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%rbp), %rcx" "\n"
    473     "movq %rcx, 1 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rax)" "\n"
    474     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ECX_OFFSET) "(%rbp), %rcx" "\n"
    475     "movq %rcx, 2 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rax)" "\n"
    476     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EBP_OFFSET) "(%rbp), %rcx" "\n"
    477     "movq %rcx, 3 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rax)" "\n"
    478     "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EIP_OFFSET) "(%rbp), %rcx" "\n"
    479     "movq %rcx, 4 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rax)" "\n"
    480     "movq %rax, %rsp" "\n"
    481 
     502
     503    // Copy remaining restore values from the ProbeContext to the restore area.
     504    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EFLAGS_OFFSET) "(%rbp), %rax" "\n"
     505    "movq %rax, 0 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rcx)" "\n"
     506    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EAX_OFFSET) "(%rbp), %rax" "\n"
     507    "movq %rax, 1 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rcx)" "\n"
     508    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_ECX_OFFSET) "(%rbp), %rax" "\n"
     509    "movq %rax, 2 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rcx)" "\n"
     510    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EBP_OFFSET) "(%rbp), %rax" "\n"
     511    "movq %rax, 3 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rcx)" "\n"
     512    "movq " STRINGIZE_VALUE_OF(PROBE_CPU_EIP_OFFSET) "(%rbp), %rax" "\n"
     513    "movq %rax, 4 * " STRINGIZE_VALUE_OF(PTR_SIZE) "(%rcx)" "\n"
     514    "movq %rcx, %rsp" "\n"
     515
     516    // Do the remaining restoration by popping off the restore area.
    482517    "popfq" "\n"
    483518    "popq %rax" "\n"
Note: See TracChangeset for help on using the changeset viewer.