Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#855 closed defect (duplicate)

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

Reported by: Mario Domenech Goulart Owned by: felix winkelmann
Priority: critical Milestone: 4.9.0
Component: core libraries Version: 4.7.x
Keywords: make-pathname Cc:
Estimated difficulty:

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)

Change History (13)

comment:1 Changed 10 years ago by Jim Ursetto

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

comment:2 Changed 10 years ago by Christian Kellermann

Owner: set to Christian Kellermann
Status: newaccepted

I have sent a patch to chicken-hackers

comment:3 Changed 10 years ago by Mario Domenech Goulart

Resolution: fixed
Status: acceptedclosed

Fixed by dd18d957edaccd5301875e6b073a873bae8b15cc

comment:4 Changed 10 years ago by Mario Domenech Goulart

Resolution: fixed
Status: closedreopened

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.

comment:5 Changed 10 years ago by Mario Domenech Goulart

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. :-)

comment:6 Changed 10 years ago by Mario Domenech Goulart

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             <--

comment:7 Changed 10 years ago by Christian Kellermann

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.

comment:8 Changed 10 years ago by Christian Kellermann

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?

comment:9 Changed 10 years ago by Christian Kellermann

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

comment:10 Changed 10 years ago by felix winkelmann

Owner: changed from Christian Kellermann to felix winkelmann
Status: reopenedassigned

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

comment:11 Changed 10 years ago by felix winkelmann

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

comment:12 Changed 10 years ago by felix winkelmann

Resolution: duplicate
Status: assignedclosed

comment:13 Changed 9 years ago by felix winkelmann

Milestone: 4.8.04.9.0

Milestone 4.8.0 deleted

Note: See TracTickets for help on using tickets.