#818 closed defect (fixed)
string-hash error
Reported by: | Jim Ursetto | Owned by: | sjamaan |
---|---|---|---|
Priority: | critical | Milestone: | 4.9.0 |
Component: | core libraries | Version: | 4.7.x |
Keywords: | Cc: | ||
Estimated difficulty: |
Description
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
Attachments (1)
Change History (6)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
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.
Changed 13 years ago by
Attachment: | 0001-Ensure-outside-hash-functions-do-not-leak-into-srfi-.patch.txt added |
---|
comment:3 Changed 13 years ago by
Owner: | set to sjamaan |
---|---|
Status: | new → assigned |
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.
comment:4 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.
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: