Skip to content

Inconsistent and missing checks in binding forms

username-removed-975496 requested to merge (removed):develop into develop

tl;dr binding forms inconsistently allowed the binding of constant variables, and progv didn't even check to see if it had a symbol before binding (which could easily lead to a segfault). My commits mostly just add some checks for both the bytecode compiler/interpreter and the C-compiler.

I gave verbose commit log messages, which are partially reproduced below, but the changes themselves are short and simple (so it may be easiest to just glance at those first).

(As mentioned in my commit log messages, I know that attempting to bind or assign to a constant variable leads to undefined behavior. One of the issues here is that ECL's behavior is inconsistent.)

There were some inconsistencies and problems with binding forms (let, let*, prog, prog*, destructuring-bind, multiple-value-bind and progv) in the bytecode compiler/interpreter and C-compiler.

For let, let*, prog, prog*, destructuring-bind and multiple-value-bind: these were allowed to bind constant variables in the bytecode compiler/interpreter, but not the C-compiler. This is inconsistent between these compilers and also inconsistent because errors are given when using the bytecode compiler/interpreter and attempting to assign to constant variables or use them in lambda lists.

> (flet ((hello () (format t "hi")))
    (let ((t nil))
      (declare (special t))
      ; Oops, now this returns a string
      (hello)))
"hi"

Changing the behavior of let and let* changes the behavior of prog, prog* and destructuring-bind.

For progv: no check for a symbol was done before attempting to bind whatever was in the variable list and no check was done for constant variables here either. Having a fixnum in the variable list caused a segfault(!) on my OpenBSD and Linux boxes (this is in both the C-compiler and bytecode compiler/interpreter). The binding of constant variables was allowed in both the C-compiler and the bytecode compiler/interpreter; however, the behavior was not consistent.

> (progv '(3) ())
Condition of type: SEGMENTATION-VIOLATION
Detected access to an invalid or protected memory address.
> (defun foo ()
    (flet ((memq (item list) (member item list :test #'eq)))
      (progv (list :test) (list :test-not)
        (memq 'bar '(bar baz quux)))))
FOO
    
> (foo)
(BAZ QUUX)
    
> (compile 'foo)
FOO
    
> (foo)
(BAR BAZ QUUX)

My commits (mostly) just add checks. Now progv checks to make sure it has a symbol before attempting to bind it, and constant variables are not allowed to be bound by these bindings forms.

I also made a change to an existing test which caused a problem during "make check" (sometimes, depending on the order of the tests). One test defined a constant variable foo and another test tried to bind foo with let. I renamed the constant foo to +foo+.

Now "make check" runs fine on my OpenBSD and Linux boxes.

I did not add any tests for these because I'm not sure if it's desired, and I haven't really looked into how to do it. Let me know if I should try to add some (if so, any hints on keeping things consistent would be appreciated).

I also did not add anything to the change log because I'm not sure if this warrants inclusion, and I'm not sure how you would want it in there anyway.

If I did anything wrong, or missed anything, then just let me know and I'll take another crack at it.

Merge request reports