Summary

`make-pathname' causes a segfault when given a list of non-strings as first argument

Metadata

Description

 
 $ csi -R files -n
 
 CHICKEN
 (c)2008-2011 The Chicken Team
 (c)2000-2007 Felix L. Winkelmann
 Version 4.7.5 (rev bd1d0fc)
 linux-unix-gnu-x86 [ manyargs dload ptables ]
 compiled 2012-05-23 on hd-t1179cl (Linux)
 
 ; loading salmonella-master/chicken/lib/chicken/6/files.import.so ...
 ; loading library files ...
 #;1> (make-pathname (list #f) "foo")
 Segmentation fault (core dumped)

Changes and comments

[2012-05-24 04:36:25 UTC] zbigniew wrote:

Confirmed here. Does not segfault on 4.7.0 or 4.7.0.5-st though.

[2012-05-24 19:18:11 UTC] ckeen changed status from new to accepted

[2012-05-24 19:18:11 UTC] ckeen set owner to ckeen

[2012-05-24 19:18:11 UTC] ckeen wrote:

I have sent a patch to chicken-hackers

[2012-05-24 20:02:09 UTC] mario changed status from accepted to closed

[2012-05-24 20:02:09 UTC] mario set resolution to fixed

[2012-05-24 20:02:09 UTC] mario wrote:

Fixed by dd18d957edaccd5301875e6b073a873bae8b15cc

[2012-05-24 22:07:51 UTC] mario changed status from closed to reopened

[2012-05-24 22:07:51 UTC] mario removed resolution fixed

[2012-05-24 22:07:51 UTC] mario wrote:

I'm not sure dd18d957edaccd5301875e6b073a873bae8b15cc is the right fix for the problem. It seems that it has been introduced between 4.7.3 and 4.7.4. In that period, the was no relevant change to files.scm.

I'm going to keep that commit as a workaround, but I think this issue deserves further investigation. Maybe we are just covering a more serious compiler bug.

[2012-05-25 19:05:13 UTC] mario wrote:

I've found (via "git bisect") that 85e8ad0baba7210e2a7cf270232af0f5388e1ef5 is the commit that triggers that behavior. More specifically, this part, probably:

 -#XXX CHICKEN_OPTIONS += -specialize -types $(SRCDIR)types.db 
 +CHICKEN_OPTIONS += -specialize -types $(SRCDIR)types.db

which seems to be just the tip of the iceberg. :-)

[2012-05-25 21:10:08 UTC] mario wrote:

Here's a code snippet that can be used to reproduce the problem (based on file.scm's conc-dirs):

 $ cat conc-dirs.scm
 (define (conc-dirs dirs)
   (let loop ((strs dirs))
     (if (null? strs)
 	""
 	(let ((s1 (car strs)))
 	  (if (zero? (string-length s1))
 	      (loop (cdr strs))
 	      (loop (cdr strs)))))))
 
 (print (conc-dirs '(#f)))



 $ csc conc-dirs.scm && ./conc-dirs
 
 Error: (string-length) bad argument type: #f
 
         Call history:
 
         conc-dirs.scm:10: conc-dirs             <--
 
 $ csc -specialize conc-dirs.scm && ./conc-dirs
 Segmentation fault (core dumped)

However, it works as expected if I just remove the let binding:

 $ cat conc-dirs2.scm
 (define (conc-dirs dirs)
   (let loop ((strs dirs))
     (if (null? strs)
 	""
 	(if (zero? (string-length (car strs)))
 	    (loop (cdr strs))
 	    (loop (cdr strs))))))
 
 (print (conc-dirs '(#f)))



 $ csc -specialize conc-dirs2.scm && ./conc-dirs2
 
 Error: (string-length) bad argument type: #f
 
         Call history:
 
         conc-dirs2.scm:9: conc-dirs             <--

[2012-05-27 19:31:23 UTC] ckeen wrote:

A simpler example to trigger this is:

 (define (t l)
   (let ((s1 (car l)))
     (zero? (string-length s1))))
 
 (print (t '(#f)))

The let in combination of the predicate zero? seems to be necessary to trigger this.

[2012-05-27 19:57:25 UTC] ckeen wrote:

The crash seems to get caused by string-length's specialisation to ##sys#size which reproducably crashes when it gets passed #f. Why does the flow analysis thing we do have a string here?

[2012-05-27 20:32:39 UTC] ckeen wrote:

I think it's the #:enforce flag of string-length's types.db entry. I have sent a patch to chicken-hackers for review.

[2012-05-28 14:45:07 UTC] felix changed status from reopened to assigned

[2012-05-28 14:45:07 UTC] felix changed owner from ckeen to felix

[2012-05-28 14:45:07 UTC] felix wrote:

This is a bug in the flow-analysis pass. Somehow the enforcement-property of `string-length` is applied to early, for whatever reason.

[2012-05-28 14:50:04 UTC] felix wrote:

The expression is walked twice. The first walk applies the enforcement, the second makes the incorrect assumption - see #751.

[2012-05-30 12:06:02 UTC] felix changed status from assigned to closed

[2012-05-30 12:06:02 UTC] felix set resolution to duplicate

[2012-09-24 21:47:48 UTC] felix changed milestone from 4.8.0 to 4.9.0

[2012-09-24 21:47:48 UTC] felix wrote:

Milestone 4.8.0 deleted