backtrace ugliness when passing invalid number of arguments to cached gf

Bug #503081 reported by Tobias C. Rittweiler
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
SBCL
Fix Released
Medium
Unassigned

Bug Description

The following code

(defgeneric gf2 (x y)
  (:method (x y)
    (+ x y)))
(defun bar2 (z)
  (gf2 z))

exhibits the following backtrace behaviour.

First we call GF2 cold:

CL-USER> (bar2 1)
; Evaluation aborted.

And we get a reasonably looking backtrace. A perfect backtrace would
include (BAR2 1) and (GF 1) -- the latter is probably optimized away
because it's a tailcall.

The function
  #<STANDARD-GENERIC-FUNCTION GF2 (1)>
requires at least 2 arguments.
   [Condition of type SB-INT:SIMPLE-PROGRAM-ERROR]

Restarts:
 0: [RETRY] Retry SLIME REPL evaluation request.
 1: [ABORT] Return to SLIME's top level.
 2: [TERMINATE-THREAD] Terminate this thread (#<THREAD "repl-thread" RUNNING {B8DEFD9}>)

Backtrace:
  0: (SB-PCL::ERROR-NEED-AT-LEAST-N-ARGS #<STANDARD-GENERIC-FUNCTION GF2 (1)> 2)
  1: (SB-PCL::INITIAL-DFUN #<STANDARD-GENERIC-FUNCTION GF2 (1)> (1))
  2: (SB-INT:SIMPLE-EVAL-IN-LEXENV (BAR2 1) #<NULL-LEXENV>)

Anyway, the problem is if we get GF2 into PCL caches:

CL-USER> (gf2 1 2)
3
CL-USER> (bar2 1)
; Evaluation aborted.

Now we get a very bad backtrace that does not only look bad, but
which is doubly bad because `v' in the debugger on the top frame
won't work, too. No fun to debug this.

invalid number of arguments: 1
   [Condition of type SB-INT:SIMPLE-PROGRAM-ERROR]

Restarts:
 0: [RETRY] Retry SLIME REPL evaluation request.
 1: [ABORT] Return to SLIME's top level.
 2: [TERMINATE-THREAD] Terminate this thread (#<THREAD "repl-thread" RUNNING {B8DEFD9}>)

Backtrace:
  0: ((LAMBDA (SB-PCL::.ARG0. SB-PCL::.ARG1.)) 1)[:EXTERNAL]
  1: (SB-INT:SIMPLE-EVAL-IN-LEXENV (BAR2 1) #<NULL-LEXENV>)

Changed in sbcl:
status: New → Confirmed
importance: Undecided → Medium
tags: added: compiler debugger pcl
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

Fixing after freeze:

diff --git a/src/pcl/methods.lisp b/src/pcl/methods.lisp
index c1e5fc4..efbd482 100644
--- a/src/pcl/methods.lisp
+++ b/src/pcl/methods.lisp
@@ -1414,7 +1414,7 @@
                         (make-dfun-lambda-list nargs applyp)
                         (make-fast-method-call-lambda-list nargs applyp))))
       (multiple-value-bind (cfunction constants)
- (get-fun1 `(lambda
+ (get-fun1 `(named-lambda (gf-dispatch ,name)
                       ,arglist
                       ,@(unless function-p
                           `((declare (ignore .pv. .next-method-call.))))

gets us:

0: ((SB-PCL::GF-DISPATCH GF2) 1) [external]
1: (SB-INT:SIMPLE-EVAL-IN-LEXENV (BAR2 1) #<NULL-LEXENV>)
2: (EVAL (BAR2 1))

In the first case GF2 isn't missing because

Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

(Nevermind the last sentence above. Stray paste.)

Changed in sbcl:
assignee: nobody → Nikodemus Siivola (nikodemus)
status: Confirmed → In Progress
Revision history for this message
Nikodemus Siivola (nikodemus) wrote :

commit 021c2d16629a00103948405549f0d70fd5ec9ad9
Author: Nikodemus Siivola <email address hidden>
Date: Sat Jan 26 23:14:02 2013 +0200

    better debug name for secondary GF dispatch functions

     Fixes lp#503081

Changed in sbcl:
assignee: Nikodemus Siivola (nikodemus) → nobody
status: In Progress → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
Revision history for this message
Douglas Katzman (dougk) wrote :

The change from LAMBDA to NAMED-LAMBDA causes the given lambda expression never (or rarely) to be structurally similar to anything already cached. This completely defeats the cache, which I think was probably unintentional.

SB-PCL> *fgens*
#<HASH-TABLE :TEST EQUAL :COUNT 21 {100034D4B3}>
SB-PCL> (handler-bind ((implicit-generic-function-warning #'muffle-warning))
 (loop for i below 500
do (eval `(defmethod ,(symbolicate "F" (write-to-string i)) ((a cons)) (list ,i a)))))
NIL
SB-PCL> *fgens*
#<HASH-TABLE :TEST EQUAL :COUNT 521 {100034D4B3}>

whereas if you back out the change, and print *fgens* before and after ...
*fgens* => #<HASH-TABLE :TEST EQUAL :COUNT 22 {100034D4B3}>
...
*fgens* => #<HASH-TABLE :TEST EQUAL :COUNT 23 {100034D4B3}>

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.