[Chicken-hackers] [PATCH] Incorrect scrutiny type for string-list and question about location c-pointer conversions

[Peter Bex]

Hi hackers!

Recently, Mario set up Salmonella so that it ran with -specialize, so
we could catch any last-minute bugs in the scrutinizer.  You can see
the results here: http://parenteses.org/mario/misc/specialize-report/

I'm happy to report that so far, most of the warnings reported by the
scrutinizer actually found hidden hidden bugs (in a dozen or so eggs).
Bugs have been filed for these eggs and several of them have already
been fixed.  Finally, there are so far only two small bugs in the
scrutinizer that we found which caused it to incorrectly show warnings.

The first bug is simple, and was found pretty quickly; eggs that use
the new(ish) foreign type specifiers "c-string-list" and "c-string-list*"
get a warning.  See here:

$ cat test.scm
(print (car ((foreign-lambda* c-string-list () "C_return(NULL);"))))
$ csc -scrutinize test.scm
Warning: at toplevel:
  (test.scm:1) in procedure call to `car', expected argument #1 of type `pair', 
but was given an argument of type `(or boolean pointer locative)'

Of course, that's incorrect since c-string-list *always* returns a list
of string objects (which can be easily seen in the code, see the
##sys#peek-c-string-list and ##sys#peek-and-free-c-string-list
procedures in library.scm:4229 and below).  The patch fixes this.

The second problem has to do with NULL pointer values in locations.
When you use let-location or define-foreign-variable and declare a
pointer type, you can set it to a NULL value by setting it to #f in
Scheme and this works correctly.  However, the scrutinizer thinks
that's bad:

$ cat test.scm
(define-foreign-variable bar c-pointer)

(set! bar #f)

(let-location ((x c-pointer #f))
  ((foreign-lambda void "printf" c-string c-pointer) "%p\n" x))
$ csc -scrutinize test.scm
Warning: at toplevel:
  (test.scm:3) in procedure call to `##sys#foreign-pointer-argument', expected 
argument #1 of type `pointer', but was given an argument of type `boolean'

Warning: at toplevel:
  (test.scm:5) in procedure call to `##sys#foreign-pointer-argument', expected 
argument #1 of type `pointer', but was given an argument of type `boolean'

This is harder to fix.  The underlying problem is that
##sys#foreign-pointer-argument is declared as accepting only
pointer types (which is correct, it will barf otherwise),
but the initializer/set value is #f, a boolean which is allowed
as a value representing a NULL pointer.  The if check in
library.scm:1077 takes care of this situation, but the
scrutinizer is unable to verify that the boolean isn't #t when
it passes the if and takes the ##sys#foreign-pointer-argument
branch (and it might be!).  This output be seen more clearly
when calling "csc -debug 2 test.scm".

We could do a few things here:

1) Add a ##core#the assertion that states the argument is
   definitely a pointer.  That kind of kills the scrutinizer's
   effectiveness at warning, moving the error to runtime (it's
   also incorrect in case anything else is passed).
2) Map #t to #f (a NULL pointer eventually in the C translation).
    That's inconsistent with the rest of core.
3) Map #t to an error.  This still moves the error to a runtime one
    but keeps the scrutinizer warnings for all non-boolean types.
4) Keep it the way it is.  That will give bogus warnings on this
    kind of code.
5) Remove ##sys#foreign-pointer-argument from types.db or change it
    to accept a boolean.  The former again gets rid of the advantage
    of having the scrutinizer and the latter is blatantly incorrect.
6) Change the ##sys#foreign-pointer-argument to accept booleans
    and only return C_SCHEME_FALSE and barf on C_SCHEME_TRUE.
7) Augment the type system with yet another special case for #f,
    adding a "false" type that is a subtype of "boolean".

I don't know which option I like most (or dislike least).
7 is definitely the most "thorough" option, similar to the special
handling of null, pair and list(-of) types, but it raises the complexity
of the scrutinizer another notch.  Other acceptable solutions would
probably either be 3 or 6, which both basically come down to the same
thing.  I think whatever we do, we should probably postpone it until
after the next release unless someone comes up with a clever, noninvasive
alternative of course :)

If we agree on this, we can make this into a Trac ticket for 4.9.0.

Cheers,
Peter
-- 
http://sjamaan.ath.cx
--
"The process of preparing programs for a digital computer
 is especially attractive, not only because it can be economically
 and scientifically rewarding, but also because it can be an aesthetic
 experience much like composing poetry or music."
                                                        -- Donald Knuth

0001-Fix-scrutiny-type-for-c-string-list-foreign-result-t.patch
Description: Text document