Save interpreter result too before anything in continuation.
authorTaylor R Campbell <campbell@mumble.net>
Wed, 2 Jan 2019 23:44:09 +0000 (23:44 +0000)
committerTaylor R Campbell <campbell@mumble.net>
Tue, 13 Aug 2019 14:37:03 +0000 (14:37 +0000)
commitfd7b4fe4c83e5e4be265cdcc1d1042f57c09ffad
tree7abe90e31aed3a15de7549a05bb693bb9c193a0b
parent987d9624b221bac31f6413ff3a6fe17b5a90a2fb
Save interpreter result too before anything in continuation.

On x86, the interpreter call result register is eax/rax, register 0,
which is also the first register we hand out for register allocation.
The continuation for an interpreter call result uses register 0, but
if the caller uses a dynamic link, the continuation first pops its
frame via the dynamic link...using a temporary register that is
guaranteed to be register 0 since it's the first one the register
allocator hands out.  The code sequence looks something like this:

;; (interpreter-call:cache-reference label-10 (register #x24) #f)
(mov q (r 2) (r 1))
(call (@ro 6 #xd0))
;; (continuation-entry label-10)
(word u #xfffc)
(block-offset label-10)
label-10:
;; (assign (register #x25) (post-increment (register 4) 1))
(pop q (r 0))
;; (assign (register #x26) (object->address (register #x25)))
(and q (r 0) (r 5))
;; (assign (offset (register 6) (machine-constant 4)) (register #x26))
(mov q (@ro 6 #x20) (r 0))
;; (assign (register #x23) (register 0))
(jmp (@pcr label-13))

On entry to the continuation, register 0 holds the value we want,
chosen as a machine alias for pseudo-register #x23 in the procedure
body, but the first thing the continuation does is pop the dynamic
link into register 0, ruining the party.

This is rather tricky to trigger because it turns out in _non-error_
cases, compiled code never asks the interpreter to evaluate a cache
reference that will return a value.  But you can trigger this by
referencing an unassigned variable and invoking a restart, which does
cause the cache reference to return a value:

;; Unassigned, so compiled code will ask interpreter for help.
(define null)

;; Recursive procedure for which the compiler uses a dynamic link.
(define (map f l)
  (let loop ((l l))
    (if (pair? l)
        (cons (f (car l)) (loop (cdr l)))
        null)))

;; Invoke the restart that will return from the cache reference with
;; a value.
(bind-condition-handler (list condition-type:unassigned-variable)
    (lambda (condition)
      condition
      (use-value '()))
  (lambda ()
    (map + '(1 2 3))))
;Value: (1 2 3 . #[false 15 #xea9c18])

Here #[false 15 #xea9c18] is the (detagged) dynamic link, a pointer
into the stack, not the result we wanted at all.
src/compiler/rtlgen/opncod.scm
src/compiler/rtlgen/rgproc.scm
src/compiler/rtlgen/rgrval.scm
src/compiler/rtlgen/rgstmt.scm
src/compiler/rtlgen/rtlgen.scm
tests/compiler/test-vartrap.scm