Steel Bank Common Lisp

block-compilation bug

Reported by Nikodemus Siivola on 2008-12-17
2
Affects Status Importance Assigned to Milestone
SBCL
High
Unassigned

Bug Description

    (let ((x 1))
      (dotimes (y 10)
        (let ((y y))
          (when (funcall (eval #'(lambda (x) (eql x 2))) y)
            (defun foo (z)
              (incf x (incf y z))))))
      (defun bar (z)
        (foo z)
        (values x)))
  (bar 1) => 11, should be 4.

Changed in sbcl:
importance: Undecided → High
status: New → Confirmed
Nikodemus Siivola (nikodemus) wrote :

Compiling this with *CHECK-CONSISTENCY* gives a hint:

strange TN live at head of #<SB-C::PHYSENV
                             :LAMBDA #<SB-C::CLAMBDA
                                       :%SOURCE-NAME SB-C::.ANONYMOUS.
                                       :%DEBUG-NAME (SB-C::TL-XEP
                                                     (LAMBDA
                                                         ()))
                                       :KIND :EXTERNAL
                                       :TYPE #<SB-KERNEL:BUILT-IN-CLASSOID
                                               FUNCTION (read-only)>
                                       :WHERE-FROM :DEFINED
                                       :VARS (#:G8)
                                       {100380C2D1}>
                             {100383B5A1}>: #<SB-C:TN Y!1>
   [Condition of type SIMPLE-ERROR]

The problem is connected with incorrect inlining of the FOO's body into BAR. To verify that, check out this code, where FOO is declared notinline:

     (declaim (notinline foo))
     (let ((x 1))
      (dotimes (y 10)
        (let ((y y))
          (when (funcall (eval #'(lambda (x) (eql x 2))) y)
            (defun foo (z)
              (incf x (incf y z))))))
      (defun bar (z)
        (foo z)
        (values x)))

(BAR 1) returns 4 in this case. You can see the problem better with this code:

(let ((x 1))
     (dotimes (y 10)
        (let ((y y))
          (when (= 2 y)
            (defun foo ()
              (print y)))))
      (defun bar () (foo)))

Try to call FOO and BAR. FOO prints 2, while BAR prints 9. The bug is that BAR has the access to Y value when inlining the FOO's body, but lexically this is wrong, since Y should not be normally visible from BAR. As a simple workaround, use 'notinline' declaration until somebody (maybe me, but I do not promise this) fixes this (probably src/compiler/locall.lisp is the right place to look into).

Roman

Nikodemus Siivola (nikodemus) wrote :

The problem lies in REFERENCE-LEAF.

Here's a tentative fix that checks that the referencer shares the parent of the referencee. It also fixes MISC.320 (meaning the current component check can go away), but I'm not yet sure it doesn't prohibit inlining in some cases where it would be legal and beneficial to do.

diff --git a/src/compiler/ir1tran.lisp b/src/compiler/ir1tran.lisp
index 728a23a..e1c1b1d 100644
--- a/src/compiler/ir1tran.lisp
+++ b/src/compiler/ir1tran.lisp
@@ -588,6 +588,20 @@

   functional)

+(defun fun-with-compatible-lexenv (fun)
+ (let ((fun (defined-fun-functional fun)))
+ (when fun
+ (let ((fun-parent (lambda-parent fun)))
+ (if fun-parent
+ (labels ((lookup (here)
+ (if (eq here fun-parent)
+ fun
+ (let ((next (lambda-parent here)))
+ (when next
+ (lookup next))))))
+ (lookup (lexenv-lambda *lexenv*)))
+ fun)))))
+
 ;;; Generate a REF node for LEAF, frobbing the LEAF structure as
 ;;; needed. If LEAF represents a defined function which has already
 ;;; been converted, and is not :NOTINLINE, then reference the
@@ -600,9 +614,8 @@
          (leaf (or (and (defined-fun-p leaf)
                         (not (eq (defined-fun-inlinep leaf)
                                  :notinline))
- (let ((functional (defined-fun-functional leaf)))
- (when (and functional
- (not (functional-kind functional))
+ (let ((functional (fun-with-compatible-lexenv leaf)))
+ (when (and functional (not (functional-kind functional))
                                      ;; Bug MISC.320: ir1-transform
                                      ;; can create a reference to a
                                      ;; inline-expanded function,

Nikodemus Siivola (nikodemus) wrote :

AARGH. Bloody launchpad killing whitespace.

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
Nikodemus Siivola (nikodemus) wrote :

In SBCL 1.0.42.25.

Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: Confirmed → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers