justinethier/cyclone

Segmentation Fault

amirouche opened this issue · 11 comments

You can reproduce with the following cli dance:

git clone --branch=amirouche-json-with-cyclone https://github.com/scheme-live/live
cd live
cyclone tests/check-0002-json-unstable.scm 
./tests/check-0002-json-unstable

The core problem here is and program exporting primitives.

For example, compiling prog.scm and running ./prog will segfault in the same way:

justin@justin-OptiPlex-790 ~/Documents/justinethier-misc/tmp $ cat prog.scm
(import
  (scheme write)
  (lib))

(write (port? 1))
justin@justin-OptiPlex-790 ~/Documents/justinethier-misc/tmp $ cat lib.sld
(define-library (lib)
  (export port? )
  (begin
    (define port? port?)))

Its also rather unfortunate (define port? port?) is even required to compile the library as built-in primitives are always implicitly available...

Put in quick fixes to prevent a segmentation fault and to no longer require define when exporting a primitive. These fixes are on the master branch.

With this change check-0002 is no longer segfaulting. However the program seems to terminate after running out of memory. Any thoughts on your end there?

What we did for other Scheme, so far, is that we disabled tests that were failing, as far as I remember, those were mostly unicode or textual representation that were different (I need to make a second pass on this).

Anyway, with cyclone the situation is different, some test files lead the runner to hang, at least the first lead to a memory leak. Here is the list of test files I need to remove, so that the test runner can finish its work:

	live/json/data/n-object-lone-continuation-byte-in-key-and-trailing-comma/input.json
	live/json/data/n-object-unterminated-value/input.json
	live/json/data/n-string-escaped-backslash-bad/input.json
	live/json/data/n-string-incomplete-escape/input.json
	live/json/data/n-string-single-doublequote/input.json
	live/json/data/n-structure-array-with-unclosed-string/input.json
	live/json/data/n-structure-open-array-open-string/input.json
        live/json/data/n-structure-open-object-open-string/input.json

When those test files are disabled via data-index.scm, the test suite pass green. I added a script the repository called ./local/bin/json2scm.scm so that you can do something like:

cyclone ./local/bin/json2scm.scm
cat live/json/data/n-structure-open-object-open-string/input.json | ./local/bin/json2scm

It seems the same bug for all those files. Do not forget to git pull the branch amirouche-json-with-cyclone.

Consider the contents of live/json/data/n-structure-open-object-open-string/input.json:

{"a

Maybe I am missing something but how is the parser expected to terminate if the check for EOF is commented out?

https://github.com/scheme-live/live/blob/hello-schemer/live/json/body.scm#L78

json-read is guarded here:

https://github.com/scheme-live/live/blob/9ff4eae00f18d1aa9ec2228524591ccb838101b2/live/json/body.scm#L273-L274

At this time, there is no precise EOF check, to speed things a little, but I could bring it back, if that is the origin of the problem

Hmm. Does that work with other Schemes such as Chibi and Chicken? I would think they would run out of memory, too. Or am I missing something?

(import (scheme base) (scheme write))                                            
                                                                                 
(define (loop lis)                                                               
  (loop (cons 1 lis)))                                                           
                                                                                 
(guard (ex (else (write `(exception ex))))                                       
  (loop 1))  

My preference is for live to check for EOF and handle appropriately.

I will bring back the eof checks.

The main branch is called hello-schemer and in that branch there is the json test suite: https://github.com/scheme-live/live/actions?query=branch%3Ahello-schemer

@amirouche Thanks! Is Cyclone passing all of your tests now?

After bringing back the eof checks it runs smoothly. I will need to update cyclone inside the docker image used in the CI.

Thanks for the help.

ref: scheme-live/live@6cfde56