0.8.0.29:
authorChristophe Rhodes <csr21@cam.ac.uk>
Tue, 3 Jun 2003 16:03:43 +0000 (16:03 +0000)
committerChristophe Rhodes <csr21@cam.ac.uk>
Tue, 3 Jun 2003 16:03:43 +0000 (16:03 +0000)
Port fix to PCL from Gerd Moellmann regarding metacircles:
there was a hole in the metacircularity detection of legacy PCL,
which is filled by bypassing ordinary slot access for standard
classes in cache miss handling when doing so would lead to
another cache miss.
... *CACHE-MISS-VALUES-STACK* and *STANDARD-SLOT-LOCATIONS* (not
audited for threadsafety)

NEWS
src/code/thread.lisp
src/pcl/dfun.lisp
src/pcl/fixup.lisp
src/pcl/std-class.lisp
src/pcl/walk.lisp
tests/mop.impure.lisp
version.lisp-expr

diff --git a/NEWS b/NEWS
index b8d55ae..da9f96c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1795,6 +1795,9 @@ changes in sbcl-0.8.1 relative to sbcl-0.8.0:
     by Teemu Kalvas)
   * bug fix: NIL is now a valid destructuring argument in DEFMACRO
     lambda lists.  (thanks to David Lichteblau)
+  * bug fix: defining a generic function with a :METHOD-CLASS being a
+    subclass of STANDARD-METHOD no longer causes stack exhaustion.
+    (thanks to Gerd Moellmann)
   * fixed some bugs revealed by Paul Dietz' test suite:
     ** NIL is now allowed as a structure slot name.
     ** arbitrary numbers, not just reals, are allowed in certain
index 288c946..5129105 100644 (file)
        ;; this punning with MAKE-LISP-OBJ depends for its safety on
        ;; the frame pointer being a lispobj-aligned integer.  While
        ;; it is, then MAKE-LISP-OBJ will always return a FIXNUM, so
-       ;; we're safe to do that.  Should this ever change, than
+       ;; we're safe to do that.  Should this ever change, this
        ;; MAKE-LISP-OBJ could return something that looks like a
        ;; pointer, but pointing into neverneverland, which will
-       ;; confuse GC compiletely.  -- CSR, 2003-06-03
+       ;; confuse GC completely.  -- CSR, 2003-06-03
        (get-mutex ,mutex (sb!kernel:make-lisp-obj (sb!sys:sap-int ,cfp))))
       (unwind-protect
           (progn ,@body)
index 106708d..066585c 100644 (file)
@@ -75,6 +75,8 @@ have to do any method lookup to implement itself.
 
 And so, we are saved.
 
+Except see also BREAK-VICIOUS-METACIRCLE.  -- CSR, 2003-05-28
+
 |#
 \f
 ;;; an alist in which each entry is of the form
@@ -171,6 +173,62 @@ And so, we are saved.
                        collect))))
            (nreverse collect)))))
 \f
+;;; Standardized class slot access: when trying to break vicious
+;;; metacircles, we need a way to get at the values of slots of some
+;;; standard classes without going through the whole meta machinery,
+;;; because that would likely enter the vicious circle again.  The
+;;; following are helper functions that short-circuit the generic
+;;; lookup machinery.
+
+(defvar *standard-classes*
+  '(standard-method standard-generic-function standard-class
+    standard-effective-slot-definition))
+
+(defvar *standard-slot-locations* (make-hash-table :test 'equal))
+
+(defun compute-standard-slot-locations ()
+  (clrhash *standard-slot-locations*)
+  (dolist (class-name *standard-classes*)
+    (let ((class (find-class class-name)))
+      (dolist (slot (class-slots class))
+       (setf (gethash (cons class (slot-definition-name slot))
+                      *standard-slot-locations*)
+             (slot-definition-location slot))))))
+
+;;; FIXME: harmonize the names between COMPUTE-STANDARD-SLOT-LOCATIONS
+;;; and MAYBE-UPDATE-STANDARD-CLASS-LOCATIONS.
+(defun maybe-update-standard-class-locations (class)
+  (when (and (eq *boot-state* 'complete)
+            (memq (class-name class) *standard-classes*))
+    (compute-standard-slot-locations)))
+
+(defun standard-slot-value (object slot-name class)
+  (let ((location (gethash (cons class slot-name) *standard-slot-locations*)))
+    (if location
+       (let ((value (if (funcallable-instance-p object)
+                        (funcallable-standard-instance-access object location)
+                        (standard-instance-access object location))))
+         (when (eq +slot-unbound+ value)
+           (error "~@<slot ~s of class ~s is unbound in object ~s~@:>"
+                  slot-name class object))
+         value)
+       (error "~@<cannot get standard value of slot ~s of class ~s ~
+                in object ~s~@:>"
+              slot-name class object))))
+
+(defun standard-slot-value/gf (gf slot-name)
+  (standard-slot-value gf slot-name *the-class-standard-generic-function*))
+
+(defun standard-slot-value/method (method slot-name)
+  (standard-slot-value method slot-name *the-class-standard-method*))
+
+(defun standard-slot-value/eslotd (slotd slot-name)
+  (standard-slot-value slotd slot-name
+                      *the-class-standard-effective-slot-definition*))
+
+(defun standard-slot-value/class (class slot-name)
+  (standard-slot-value class slot-name *the-class-standard-class*))
+\f
 ;;; When all the methods of a generic function are automatically
 ;;; generated reader or writer methods a number of special
 ;;; optimizations are possible. These are important because of the
@@ -1020,6 +1078,8 @@ And so, we are saved.
 ;;;  <index>      If <type> is READER or WRITER, and the slot accessed is
 ;;;           an :instance slot, this is the index number of that slot
 ;;;           in the object argument.
+(defvar *cache-miss-values-stack* ())
+
 (defun cache-miss-values (gf args state)
   (multiple-value-bind (nreq applyp metatypes nkeys arg-info)
       (get-generic-fun-info gf)
@@ -1036,26 +1096,100 @@ And so, we are saved.
                 accessor-type index)))))
 
 (defun cache-miss-values-internal (gf arg-info wrappers classes types state)
+  (if (and classes (equal classes (cdr (assq gf *cache-miss-values-stack*))))
+      (break-vicious-metacircle gf classes arg-info)
+      (let ((*cache-miss-values-stack*
+            (acons gf classes *cache-miss-values-stack*))
+           (cam-std-p (or (null arg-info)
+                          (gf-info-c-a-m-emf-std-p arg-info))))
+       (multiple-value-bind (methods all-applicable-and-sorted-p)
+           (if cam-std-p
+               (compute-applicable-methods-using-types gf types)
+               (compute-applicable-methods-using-classes gf classes))
+         
   (let* ((for-accessor-p (eq state 'accessor))
         (for-cache-p (or (eq state 'caching) (eq state 'accessor)))
-        (cam-std-p (or (null arg-info)
-                       (gf-info-c-a-m-emf-std-p arg-info))))
-    (multiple-value-bind (methods all-applicable-and-sorted-p)
-       (if cam-std-p
-           (compute-applicable-methods-using-types gf types)
-           (compute-applicable-methods-using-classes gf classes))
-      (let ((emf (if (or cam-std-p all-applicable-and-sorted-p)
-                    (function-funcall (get-secondary-dispatch-function1
-                                       gf methods types nil (and for-cache-p
-                                                                 wrappers)
-                                       all-applicable-and-sorted-p)
-                                      nil (and for-cache-p wrappers))
-                    (default-secondary-dispatch-function gf))))
-       (multiple-value-bind (index accessor-type)
-           (and for-accessor-p all-applicable-and-sorted-p methods
-                (accessor-values gf arg-info classes methods))
-         (values (if (integerp index) index emf)
-                 methods accessor-type index))))))
+        (emf (if (or cam-std-p all-applicable-and-sorted-p)
+                 (function-funcall (get-secondary-dispatch-function1
+                                    gf methods types nil (and for-cache-p
+                                                              wrappers)
+                                    all-applicable-and-sorted-p)
+                                   nil (and for-cache-p wrappers))
+                 (default-secondary-dispatch-function gf))))
+    (multiple-value-bind (index accessor-type)
+       (and for-accessor-p all-applicable-and-sorted-p methods
+            (accessor-values gf arg-info classes methods))
+      (values (if (integerp index) index emf)
+             methods accessor-type index)))))))
+
+;;; Try to break a vicious circle while computing a cache miss.
+;;; GF is the generic function, CLASSES are the classes of actual
+;;; arguments, and ARG-INFO is the generic functions' arg-info.
+;;;
+;;; A vicious circle can be entered when the computation of the cache
+;;; miss values itself depends on the values being computed.  For
+;;; instance, adding a method which is an instance of a subclass of
+;;; STANDARD-METHOD leads to cache misses for slot accessors of
+;;; STANDARD-METHOD like METHOD-SPECIALIZERS, and METHOD-SPECIALIZERS
+;;; is itself used while we compute cache miss values.
+(defun break-vicious-metacircle (gf classes arg-info)
+  (when (typep gf 'standard-generic-function)
+    (multiple-value-bind (class slotd accessor-type)
+       (accesses-standard-class-slot-p gf)
+      (when class
+       (let ((method (find-standard-class-accessor-method
+                      gf class accessor-type))
+             (index (standard-slot-value/eslotd slotd 'location))
+             (type (gf-info-simple-accessor-type arg-info)))
+         (when (and method
+                    (subtypep (ecase accessor-type
+                                ((reader) (car classes))
+                                ((writer) (cadr classes)))
+                              class))
+           (return-from break-vicious-metacircle
+             (values index (list method) type index)))))))
+  (error "~@<vicious metacircle:  The computation of an ~
+         effective method of ~s for arguments of types ~s uses ~
+         the effective method being computed.~@:>"
+        gf classes))
+
+;;; Return (CLASS SLOTD ACCESSOR-TYPE) if some method of generic
+;;; function GF accesses a slot of some class in *STANDARD-CLASSES*.
+;;; CLASS is the class accessed, SLOTD is the effective slot definition
+;;; object of the slot accessed, and ACCESSOR-TYPE is one of the symbols
+;;; READER or WRITER describing the slot access.
+(defun accesses-standard-class-slot-p (gf)
+  (flet ((standard-class-slot-access (gf class)
+          (loop with gf-name = (standard-slot-value/gf gf 'name)
+                for slotd in (standard-slot-value/class class 'slots)
+                ;; FIXME: where does BOUNDP fit in here?  Is it
+                ;; relevant?
+                as readers = (standard-slot-value/eslotd slotd 'readers)
+                as writers = (standard-slot-value/eslotd slotd 'writers)
+                if (member gf-name readers :test #'equal)
+                  return (values slotd 'reader)
+                else if (member gf-name writers :test #'equal)
+                  return (values slotd 'writer))))
+    (dolist (class-name *standard-classes*)
+      (let ((class (find-class class-name)))
+       (multiple-value-bind (slotd accessor-type)
+           (standard-class-slot-access gf class)
+         (when slotd
+           (return (values class slotd accessor-type))))))))
+
+;;; Find a slot reader/writer method among the methods of generic
+;;; function GF which reads/writes instances of class CLASS.
+;;; TYPE is one of the symbols READER or WRITER.
+(defun find-standard-class-accessor-method (gf class type)
+  (dolist (method (standard-slot-value/gf gf 'methods))
+    (let ((specializers (standard-slot-value/method method 'specializers))
+         (qualifiers (plist-value method 'qualifiers)))
+      (when (and (null qualifiers)
+                (eq (ecase type
+                      (reader (car specializers))
+                      (writer (cadr specializers)))
+                    class))
+       (return method)))))
 
 (defun accessor-values (gf arg-info classes methods)
   (declare (ignore gf))
index b759071..e51da94 100644 (file)
@@ -24,6 +24,7 @@
 (in-package "SB-PCL")
 
 (!fix-early-generic-functions)
+(compute-standard-slot-locations)
 (setq *boot-state* 'complete)
 
 (defun print-std-instance (instance stream depth)
index b2d4cb7..22c3c10 100644 (file)
              wrapper nwrapper))
       (setf (slot-value class 'finalized-p) t)
       (unless (eq owrapper nwrapper)
-       (update-pv-table-cache-info class)))))
+       (update-pv-table-cache-info class)
+       (maybe-update-standard-class-locations class)))))
 
 (defun compute-class-slots (eslotds)
   (let (collect)
index 2dbeb0c..bca2bda 100644 (file)
 
 (defmacro with-rpush (&body body)
   `(with-lexical-macros ,(list (list 'rpush #'expand-rpush)) ,@body))
-|#
\ No newline at end of file
+|#
index e010957..d5116f9 100644 (file)
 ;;; 2003-04-17
 (assert (> *compute-effective-slot-definition-count* 0))
 \f
+;;; this used to cause a nasty uncaught metacircularity in PCL.
+(defclass substandard-method (standard-method) ())
+(defgeneric substandard-defgeneric (x y)
+  (:method-class substandard-method)
+  (:method ((x number) (y number)) (+ x y))
+  (:method ((x string) (y string)) (concatenate 'string x y)))
+(assert (= (substandard-defgeneric 1 2) 3))
+(assert (string= (substandard-defgeneric "1" "2") "12"))
+\f
 ;;;; success
 (sb-ext:quit :unix-status 104)
index 5cec751..8ffe15d 100644 (file)
@@ -17,4 +17,4 @@
 ;;; checkins which aren't released. (And occasionally for internal
 ;;; versions, especially for internal versions off the main CVS
 ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".)
-"0.8.0.28"
+"0.8.0.29"