string-hash error

[zbigniew]

This error has been causing failure in sql-de-lite, fps, and hashes tests on salmonella since 2012/01/06. It succeeded on salmonella 2012/01/04. Both runs are reported to be in Chicken 4.7.4 series but it is not known yet exactly which sha1 hash.

Error: (string-parse-start+end) Illegal substring START spec
#<procedure (string-hash s1126 . maybe-bound+start+end1127)>
#f

IRC transcript:

17:16:36 < zbigniew> http://tests.call-cc.org/master/linux/x86/2012/04/14/salmonella-report/test/sql-de-lite.html
17:18:21 < sjamaan> What exactly is it doing?
17:22:10 < zbigniew> hash-table-ref of a string=? hash table, probably via lru-cache
17:22:17 < zbigniew> failure beings here: http://tests.call-cc.org/master/linux/x86/2012/01/06/salmonella-report/tests/sql-de-lite.html
17:22:28 < zbigniew> begins
17:22:33 < zbigniew> ok here: http://tests.call-cc.org/master/linux/x86/2012/01/04/salmonella-report/tests/sql-de-lite.html
17:23:49 < sjamaan> Can't reproduce with a simple hash-table test
17:27:24 < zbigniew> other eggs are apparently having the same issue
17:27:25 < zbigniew> http://tests.call-cc.org/master/linux/x86/2012/04/14/salmonella-report/test/fps.html
17:28:03 < zbigniew> http://tests.call-cc.org/master/linux/x86/2012/04/14/salmonella-report/test/hashes.html (at end of tests)
17:28:28 < sjamaan> So this has been broken for months. Why didn't anyone report this?
17:29:07 < zbigniew> probably no one affected is using master
17:29:18 < zbigniew> nomads, but it's because it depends on sql-de-lite, so that doesn't count
17:31:40 < zbigniew> all right, ultimately, it looks like only fps and sql-de-lite are having a problem; hashes counts but has a bunch of other problems
17:32:10 < zbigniew> i assume sql-de-lite's problem is due to lru-cache

[zbigniew]

Believe it or not, this seems to be related to the posix egg. Any string=? hash table created after posix is loaded will fail. Try the following script in csi:

(use srfi-69)
(define h (make-hash-table string=?))
(define h2 (make-hash-table string=?))
(use posix)
(define h3 (make-hash-table string=?))
(hash-table-set! h "foo" 1)
(hash-table-set! h2 "bar" 2)
(hash-table-set! h3 "baz" 3)           ;; error

Error: (string-parse-start+end) Illegal substring START spec
#<procedure (string-hash s1126 . maybe-bound+start+end1127)>
#f
"baz"

[zbigniew]

Indirectly only. It's actually an interaction with srfi-13.

(use srfi-69 srfi-13) and the errors will occur.
(use srfi-13 srfi-69) and they will not.

srfi-13's string-hash is apparently leaking and conflicting with srfi-69's definition. My assumption is that this has always happened, as srfi-69 takes no precautions in make-hash-table or *make-hash-function to ensure that its own version of string-hash is being used when string=? is specified. However, now srfi-13's version does not comply with the required API.

Ultimately this was introduced by two commits, the first of which coincides with the beginning of test failures, 2012/01/04:
2d722205ee1d827d1555761df72f330519c6c1c5, which changed the signature of string-hash to add a randomization parameter
and the second of which coincides with a change in the error format, 2012/02/01:
5ddfa715e50a6bf9c117d7c3bbd17298a6d8061a, which uses #f to indicate missing start/end parameters

make-hash-table and *make-hash-function identify "string-hash" as being the built-in (i.e. non-user) version even when srfi-13's version has overridden it, as they just do a global lookup at runtime. They then attempt to pass it #f/#f for start/end. Unfortunately, srfi-13 doesn't accept #f for these parameters, as it uses let-optionals. Hence, the error.

Prior to the second commit, it did not pass #f/#f/seed to string-hash, but simply a randomization seed. Indeed, the test output reflects this change and between 2012/01/04 and 2012/02/01 the error was different:

Error: (string-parse-start+end) Illegal substring START/END spec
#<procedure (string-hash s1126 . maybe-bound+start+end1127)>
306135522
15

Probably the fix is to use a LET around the definition of *make-hash-function and make-hash-table to avoid the runtime lookup and ensure srfi-69's versions are the ones checked for.

[zbigniew]

Attached patch fixes the problem. You can see now the hash function is correct.

before:

$ csi -R srfi-69 -R srfi-13
#;1> (hash-table-hash-function (make-hash-table string=?))
#<procedure (string-hash s1128 . maybe-bound+start+end1129)>

after:

$ csi -R srfi-69 -R srfi-13
#;1> (hash-table-hash-function (make-hash-table string=?))
#<procedure (string-hash str608 . tmp607609)>

I verified that older chicken (for example, 4.7.0.5-st) also uses the srfi-13 hash function. However it was never noticed before as the API was identical.

It might be a good idea to add keyword-hash, symbol-hash and object-uid-hash to *make-hash-function to enable randomization? Not sure why they are missing. If this sounds ok then I will update this patch.

[sjamaan]

Patch looks good, applied.

I agree adding keyword-hash, symbol-hash and object-uid-hash should be added for randomization. It's a stupid oversight on my part. I'll send a patch to do that to chicken-hackers in the weekend unless someone else beats me to it.

[felix]

Milestone 4.8.0 deleted