Opened 6 years ago

Closed 6 years ago

#1317 closed defect (fixed)

LOAD-TIME-VALUE constantness issue

Reported by: ivan4th Owned by: gb
Priority: normal Milestone:
Component: other Version: trunk
Keywords: Cc:

Description

The following code works fine in ccl-1.9:

(in-package :cl-user)

(defun abc ()
  (let* ((body
           '(let ((cache (load-time-value (cons (cons nil nil) nil))))
             (let ((fn #'(lambda () t)))
               (setf (car cache) (cons fn t))
               (values (funcall (caar cache))))))
         (thunk (compile nil `(lambda () ,body))))
    (disassemble thunk)
    (funcall thunk)))

(print (abc))

but fails with "Error of type CCL::INVALID-MEMORY-ACCESS: Fault during read of memory address #x0" error on (funcall (caar cache)) form in trunk and ccl-1.10.

The code was distilled from a piece of code utilizing LOAD-TIME-VALUE cache pattern as in http://random-state.net/log/3507100003.html (I used it in a compiler macro)

The problem can be currently worked around by commenting out (multiple-value-bind ...) form in (def-acode-rewrite acode-rewrite-cxr ...) in compiler/acode-rewrite.lisp: http://trac.clozure.com/ccl/browser/trunk/source/compiler/acode-rewrite.lisp?rev=16571#L567

Disassembly output before applying the workaround. Looks like the compiler treats (load-time-value (cons (cons nil nil) nil)) as a read-only constant value that cannot be changed:

    (recover-fn-from-rip)                   ;     [7]
    (testl (% nargs) (% nargs))             ;    [14]
    (jne L149)                              ;    [16]
    (pushq (% rbp))                         ;    [22]
    (movq (% rsp) (% rbp))                  ;    [23]
    (movq (@ '#<Anonymous Function #x302000B53A9F> (% fn)) (% arg_z)) ;    [26]
    (pushq (% arg_z))                       ;    [33]
    (movq (@ '((NIL)) (% fn)) (% arg_y))    ;    [34]
    (pushq (% arg_y))                       ;    [41]
    (movq (@ -8 (% rbp)) (% arg_y))         ;    [42]
    (movl ($ #x1302E) (% arg_z.l))          ;    [46]
    (subq ($ 13) (@ (% gs) #xD8))           ;    [51]
    (movq (@ (% gs) #xD8) (% temp0))        ;    [61]
    (cmpq (@ (% gs) #xE0) (% temp0))        ;    [70]
    (ja L76)                                ;    [79]
    (uuo-alloc)                             ;    [81]
L76
    (andb ($ #xF0) (@ (% gs) #xD8))         ;    [83]
    (movq (% arg_y) (@ 5 (% temp0)))        ;    [92]
    (movq (% arg_z) (@ -3 (% temp0)))       ;    [96]
    (movq (% temp0) (% arg_z))              ;   [100]
    (popq (% arg_y))                        ;   [103]
    (lisp-call  (@ .SPRPLACA))              ;   [109]
    (recover-fn-from-rip)                   ;   [116]
    (xorl (% nargs) (% nargs))              ;   [123]
    (movq (@ 'NIL (% fn)) (% temp0))        ;   [125]
    (lisp-call (@ 10 (% temp0)))            ;   [137]
    (recover-fn-from-rip)                   ;   [140]
    (leaveq)                                ;   [147]
    (retq)                                  ;   [148]
L149
    (uuo-error-wrong-number-of-args)

Disassembly output after applying the workaround:

    (recover-fn-from-rip)                   ;     [7]
    (testl (% nargs) (% nargs))             ;    [14]
    (jne L193)                              ;    [16]
    (pushq (% rbp))                         ;    [22]
    (movq (% rsp) (% rbp))                  ;    [23]
    (movq (@ '#<Anonymous Function #x302000B0A71F> (% fn)) (% arg_z)) ;    [26]
    (pushq (% arg_z))                       ;    [33]
    (movq (@ '((NIL)) (% fn)) (% arg_y))    ;    [34]
    (pushq (% arg_y))                       ;    [41]
    (movq (@ -8 (% rbp)) (% arg_y))         ;    [42]
    (movl ($ #x1302E) (% arg_z.l))          ;    [46]
    (subq ($ 13) (@ (% gs) #xD8))           ;    [51]
    (movq (@ (% gs) #xD8) (% temp0))        ;    [61]
    (cmpq (@ (% gs) #xE0) (% temp0))        ;    [70]
    (ja L76)                                ;    [79]
    (uuo-alloc)                             ;    [81]
L76 
    (andb ($ #xF0) (@ (% gs) #xD8))         ;    [83]
    (movq (% arg_y) (@ 5 (% temp0)))        ;    [92]
    (movq (% arg_z) (@ -3 (% temp0)))       ;    [96]
    (movq (% temp0) (% arg_z))              ;   [100]
    (popq (% arg_y))                        ;   [103]
    (lisp-call  (@ .SPRPLACA))              ;   [109]
    (recover-fn-from-rip)                   ;   [116]
    (movq (@ '((NIL)) (% fn)) (% arg_z))    ;   [123]
    (movq (@ 5 (% arg_z)) (% arg_z))        ;   [130]
    (movl (% arg_z.l) (% imm0.l))           ;   [134]
    (andl ($ 7) (% imm0.l))                 ;   [136]
    (cmpl ($ 3) (% imm0.l))                 ;   [139]
    (jne L201)                              ;   [142]
    (movq (@ 5 (% arg_z)) (% arg_z))        ;   [144]
    (pushq (% arg_z))                       ;   [148]
    (movq (@ -16 (% rbp)) (% temp0))        ;   [149]
    (xorl (% nargs) (% nargs))              ;   [153]
    (movl (% temp0.l) (% imm0.l))           ;   [155]
    (andl ($ 15) (% imm0.l))                ;   [157]
    (cmpl ($ 14) (% imm0.l))                ;   [160]
    (cmovgq (% temp0) (% temp1))            ;   [163]
    (jl L209)                               ;   [167]
    (cmoveq (@ 10 (% temp0)) (% temp1))     ;   [169]
    (lisp-call (% temp1))                   ;   [177]
    (recover-fn-from-rip)                   ;   [180]
    (addq ($ 8) (% rsp))                    ;   [187]
    (leaveq)                                ;   [191]
    (retq)                                  ;   [192]
L193
    (uuo-error-wrong-number-of-args)        ;   [200]
L201
    (uuo-error-reg-not-list (% arg_z))      ;   [208]
L209
    (uuo-error-not-callable)                ;   [216]

Change History (4)

comment:1 Changed 6 years ago by gb

  • Owner set to gb
  • Status changed from new to assigned

comment:2 Changed 6 years ago by gb

The code that you are commenting out is trying to say that CAR (or CDR) of a constant list (a literal object) cam be done at compile-time, unless the "constant list" was created by COMPILE-FILE's processing of a LOAD-TIME-VALUE form. So (CAR '(A B) can just compile to a reference to A, and that will always return the same value.

If a LOAD-TIME_VALUE form is processed by COMPILE-FILE, it might be incorrect to handle some things this way.

In your example, the LOAD-TIME-VALUE is processed by COMPILE (at compile-time), and the value is treated as a literal object

(let ((cache '((nil))  
...
  (setf (car cache) ...) destructive modification of a literal object has unspecified consequences

COMPILE-FILE can't treat things defined via LOAD-TIME-VALUE as being read-only or coalesce similar values into a single constant, but COMPILE (and EVAL) are required to treat such things as "literal objects". I think that the behavior that you are seeing is correct but would not be if the LOAD-TIME-VALUE form had been processed by COMPILE-FILE.

The distinction between "constant objects" and "literal objects" is a subtle one, and I'm not sure that the (small) benefits of being able to process (CAR '(A B C)) at compile-time justify the confusion that doing so causes.

CLHS says that the value produced by LOAD-TIME-VALUE must be considered to be "potentially mutable." it is clear (to me) what this means " ... when COMPILE-FILE processes the LOAD-TIME-VALUE form", but the spec doesn't say what "potentially mutable" means in other contexts, and it is often better to avoid inferring intent when interpreting a language spec,

Note also that the code that you linked to differs from the code in your report in that your code calls COMPILE explicitly. and COMPILE is required to process LOAD-TIME-VALUE at compile-time and treat the result as a literal constant in compiled code. I would expect the code that you linked to to work as expected.

Interpreting the "potentially mutable" phrase as applying in all cases is not unreasonable, but neither is treating the form as a literal constant, and these behaviors seem contradictory.

At the moment. I lean towards reverting to the old behavior and avoiding the issue.

comment:3 Changed 6 years ago by gb

Never mind. CCL -has- a mechanism for distinguishing literal objects produced by COMPILE's handling of LOAD-TIME-VALUE from other literal objects, but it is not fully implemented, and I think that it needs to be.

comment:4 Changed 6 years ago by gb

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [16611]) handle LOAD-TIME-VALUE differently. In the COMPILE (EVAL) case, wrap the literal (immediate) in new acode. make ACODE-CONSTANT-P recognize the COMPILE-FILE case, and return NIl,NIL Fixes ticket:1317 in the trunk

Note: See TracTickets for help on using tickets.