Re: [PATCH] Fix missing argument for barf on out-of-range error (u8vector-set!)

[Peter Bex]

On Fri, Sep 27, 2024 at 10:45:52PM +0200, Mario Domenech Goulart wrote:
> Hi,
>
> Attached is a patch to fix the call to barf when handling u8vector-set!
> out-of-range errors.  Reported by puck in #chicken.

Hi Mario,

This patch works, but it led me down a rabbit hole.

So, as you can see, in runtime.c we have a case for C_OUT_OF_RANGE_ERROR
in barf() which expects 4 arguments.  That's the error number, the
location as a string and two extra arguments which are (I think)
intended to be a "context" and the irritant.  For instance, if you
call (u8vector-set! #u8() 2 0), it complains and gives you the vector
(context) and the index in the "arguments" property of the condition.

This then is put on the stack and handed of too ##sys#error-hook to
construct the condition and call the exception handler.

But now take a look at the definition of ##sys#check-range, the
Scheme procedure which AFAICT is the only one which corresponds
directly to such barf calls:

    (define (##sys#check-range i from to . loc)
      (##sys#check-fixnum i loc)
      (unless (and (fx<= from i) (fx< i to))
        (##sys#error-hook
         (foreign-value "C_OUT_OF_RANGE_ERROR" int)
         (and (pair? loc) (car loc)) i from to) ) )

This calls ##sys#error-hook with the error code (C_OUT_OF_RANGE_ERROR,
the value of which is 8) as the first argument, and either #f or the
location as the second argument and "i" (the index being checked, which
is the "irritant") as the third argument.  Then it passes "from" and
"to" as the fourth and fifth arguments for "context", respectively.

This means the condition will look differently when it's raised directly
from Scheme.  Compare:

    #;1> (import srfi-4)
    #;2> (u8vector-set! #u8() 1 300)
    
    Error: (u8vector-set!) out of range
    #u8()
    1
    #;2> ,exn
    condition: (exn bounds)
     exn
            message: "out of range"
            arguments: (#u8() 1)
            call-chain: (#("<stdin>:3" (u8vector-set! #u8() 1 300) #f) 
#("<stdin>:3" (u8vector-set! #u8() 1 300) #...
            location: u8vector-set!
     bounds

With:

    #;1> (subvector #() 1 2)
    
    Error: (subvector) out of range
    1
    0
    1
    #;1> ,exn
    condition: (exn bounds)
     exn
            message: "out of range"
            arguments: (1 0 1)
            call-chain: (#("<stdin>:1" (subvector #() 1 2) #f) #("<stdin>:1" 
(subvector #() 1 2) #<frameinfo>))
            location: subvector
     bounds

I think we should think about how to ensure the C calls match the Scheme
calls, and bring those in line.

Note that the definitions for check-uint-length and check-int-length in
srfi-4.scm match up with ##sys#check-range - they pass the irritant and
the from/to values to ##sys#error-hook.

There is also a direct ##sys#error-hook call with C_OUT_OF_RANGE_ERROR
in substring-index(-ci) in data-structures.scm and scheme#substring in
library.scm also seems to be missing one argument.

There are quite a few places in runtime.c where we call
C_OUT_OF_RANGE_ERROR with the context and the irritant alone, we'd have
to change all of those to call it with the irritant, min and max value.

I think we'd do best to completely replace all barf(C_OUT_OF_RANGE, ...)
calls in a bigger patch and make it use 5 instead of 4 arguments.

But for now, attached is a small modified version of your patch which
brings u8vector-set!  in line with s8vector-set! and the f32 and f64vector
setters, which use C_BAD_ARGUMENT_TYPE_ERROR.  I also fixed the other
srfi-4 setters which had this incorrect call.

On the longer term, I think it's probably better to completely refactor
barf() and ##sys#error-hook.  Currently there's a lot of duplication in
the error string, the foreign integer values have to be duplicated
as literals in the case expression and the argument counts need
to line up correctly.  As we've seen this is not easy to get
right, resulting in inconsistencies in the structure of the condition
objects that get constructed.  This would definitely be a problem
for code that tries to inspect such condition objects.

Cheers,
Peter

0001-Fix-barf-calls-for-out-of-range-values-for-srfi-4-se.patch
Description: Text document