justinethier / cyclone

:cyclone: A brand-new compiler that allows practical application development using R7RS Scheme. We provide modern features and a stable system capable of generating fast native binaries.
http://justinethier.github.io/cyclone/
MIT License
823 stars 42 forks source link

Segmentation fault when handling bytevectors #392

Closed arthurmaciel closed 4 years ago

arthurmaciel commented 4 years ago

@justinethier, I always remember you saying that Cyclone should not stop by segmentation faults. When porting Chibi's bytevector library, one of its tests breaks the system. The culprit is this commented test, specifically due to line 65. When setting the bv to 0.0 it works ok. But when trying to set it to any other value I get the segfault.

You can reproduce it by running:

cyclone-winds retrieve bytevector
cd bytevector
cyclone-winds build-local
# Uncomment the culprit test on test.scm and...
cyclone-winds test-local

For me it produces a test.core and when I try to debug with

$ gdb test test.core

I get:

Reading symbols from test...
[New LWP 100866]
[New LWP 101425]
Core was generated by `./test'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00000000003f67f4 in mp_get_double ()
[Current thread is 1 (LWP 100866)]
(gdb)

Unfortunately I am not skilled enough to propose a PR to solve this issue.

justinethier commented 4 years ago

@arthurmaciel thanks for the report! Right, the runtime and our compiled programs should never segfault.

Here is what gdb says, looks like a comparison between a double and a bignum is leading to a crash:

(gdb) run
Starting program: /home/justin/Documents/tmp/bytevector/test
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7ffff64a2700 (LWP 3176)]
bytevector:
    reading ieee: ........................
    24 out of 24 (100.0%) tests passed in 0.0 seconds.
    writing ieee: .............
Thread 1 "test" received signal SIGSEGV, Segmentation fault.
0x000055555584ab2a in mp_get_double ()
(gdb) bt
#0  0x000055555584ab2a in mp_get_double ()
#1  0x00005555557c9b45 in Cyc_num_fast_gte_op (data=0x555555a93790, x=0x7fffffff9e00,
    y=0x7ffff6ab7440) at runtime.c:1846
#2  0x00005555556bad04 in __lambda_263 (data=0x555555a93790, argc=3,
    self_733632=0x7ffff64a3240, k_733147=0x7ffff64a32a0, n_731122_732500=0x7fffffff9e00,
    e_731123_732501=0x829) at cyclone/bytevector.c:3167
#3  0x00005555557fcec8 in do_dispatch (data=0x555555a93790, argc=3,
    func=0x5555556bac95 <__lambda_263>, clo=0x7ffff64a3240, b=0x555555a93de0) at dispatch.c:6
#4  0x00005555557eefa4 in Cyc_start_trampoline (thd=0x555555a93790) at runtime.c:5742
#5  0x000055555557d706 in main (argc=1, argv=0x7fffffffe5f8, envp=0x7fffffffe608)
    at test.c:3482
(gdb) frame 1
#1  0x00005555557c9b45 in Cyc_num_fast_gte_op (data=0x555555a93790, x=0x7fffffff9e00,
    y=0x7ffff6ab7440) at runtime.c:1846
1846    declare_num_cmp(Cyc_num_gte, Cyc_num_gte_op, Cyc_num_fast_gte_op, dispatch_num_gte, >=, CYC_BN_GTE);
(gdb) p x
$1 = (object) 0x7fffffff9e00
(gdb) p *((list)x)
$2 = {hdr = {mark = 0 '\000', grayed = 0 '\000', immutable = 1 '\001'}, tag = 9 '\t',
  pair_car = 0x40000000, pair_cdr = 0x100000001}
(gdb) p *((list)y)
$3 = {hdr = {mark = 3 '\003', grayed = 0 '\000', immutable = 0 '\000'}, tag = 13 '\r',
  pair_car = 0x2000000001, pair_cdr = 0x0}
justinethier commented 4 years ago

The bug is in call-with-mantissa&exponent. To reproduce:

(call-with-mantissa&exponent 1.0 2 52 11 list)

Will need to investigate further later today; but it is good we can isolate it.

arthurmaciel commented 4 years ago

@arthurmaciel thanks for the report! Right, the runtime and our compiled programs should never segfault.

Here is what gdb says, looks like a comparison between a double and a bignum is leading to a crash:

(gdb) run
Starting program: /home/justin/Documents/tmp/bytevector/test
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7ffff64a2700 (LWP 3176)]
bytevector:
    reading ieee: ........................
    24 out of 24 (100.0%) tests passed in 0.0 seconds.
    writing ieee: .............
Thread 1 "test" received signal SIGSEGV, Segmentation fault.
0x000055555584ab2a in mp_get_double ()
(gdb) bt
#0  0x000055555584ab2a in mp_get_double ()
#1  0x00005555557c9b45 in Cyc_num_fast_gte_op (data=0x555555a93790, x=0x7fffffff9e00,
    y=0x7ffff6ab7440) at runtime.c:1846
#2  0x00005555556bad04 in __lambda_263 (data=0x555555a93790, argc=3,
    self_733632=0x7ffff64a3240, k_733147=0x7ffff64a32a0, n_731122_732500=0x7fffffff9e00,
    e_731123_732501=0x829) at cyclone/bytevector.c:3167
#3  0x00005555557fcec8 in do_dispatch (data=0x555555a93790, argc=3,
    func=0x5555556bac95 <__lambda_263>, clo=0x7ffff64a3240, b=0x555555a93de0) at dispatch.c:6
#4  0x00005555557eefa4 in Cyc_start_trampoline (thd=0x555555a93790) at runtime.c:5742
#5  0x000055555557d706 in main (argc=1, argv=0x7fffffffe5f8, envp=0x7fffffffe608)
    at test.c:3482
(gdb) frame 1
#1  0x00005555557c9b45 in Cyc_num_fast_gte_op (data=0x555555a93790, x=0x7fffffff9e00,
    y=0x7ffff6ab7440) at runtime.c:1846
1846    declare_num_cmp(Cyc_num_gte, Cyc_num_gte_op, Cyc_num_fast_gte_op, dispatch_num_gte, >=, CYC_BN_GTE);
(gdb) p x
$1 = (object) 0x7fffffff9e00
(gdb) p *((list)x)
$2 = {hdr = {mark = 0 '\000', grayed = 0 '\000', immutable = 1 '\001'}, tag = 9 '\t',
  pair_car = 0x40000000, pair_cdr = 0x100000001}
(gdb) p *((list)y)
$3 = {hdr = {mark = 3 '\003', grayed = 0 '\000', immutable = 0 '\000'}, tag = 13 '\r',
  pair_car = 0x2000000001, pair_cdr = 0x0}

@justinethier, thanks for the lesson on debugging the C code. I would never figure it out myself. Just linking the enum object_tag for further reference if anyone is interested.

justinethier commented 4 years ago

There is one bug here, where we are comparing x to x instead of y:

https://github.com/justinethier/cyclone/blob/master/runtime.c#L1821

Not sure if this explains the segfault though...

Here is a simplified program to repro the crash:

(import (scheme base) (scheme write))
(define i 0)
(define (call-with-mantissa&exponent num base mant-size exp-size proc)
    (let* ((bot (expt base mant-size))
           (top (* base bot)))
      (let loop ((n (inexact num)) (e 0))
        (write `(DEBUG ,n ,(inexact top) ,top ,i)) (newline) (flush-output-port)
        (set! i (+ i 1))
        (cond
         ((<= n top)
          (write `(DEBUG2 ,(>= n top) ,n ,i)) (newline) (flush-output-port)
          (loop (/ n base) (+ e 1)))
         ))))
(write
 (call-with-mantissa&exponent 1.0 2 52 11 list))
justinethier commented 4 years ago

@arthurmaciel so there was just the single issue here. The problem was that x (a double) was being dereferenced as a bignum which lead to an invalid return value for the comparison and could lead to undefined behavior/crashes depending upon whether valid memory was accessed:

    } else if (tx == double_tag && ty == bignum_tag) { \
      return (double_value(x)) OP mp_get_double(&bignum_value(x)) ? boolean_t : boolean_f; \

This is fixed in master.

Some bad news though - the floating point tests compile and run now but seem to be failing. Should we open another ticket for them or are you still in the process of porting / testing this?

arthurmaciel commented 4 years ago

Really nice how you precisely spot the bug. I will check what is happening with the floating point tests. This shouldn't happen. Thanks!

arthurmaciel commented 4 years ago

@justinethier, can you reproduce the following?

$ sudo cyclone-winds reinstall bytevector

$ rlwrap icyc 

              :@ 
            @@@  
          @@@@:  
        `@@@@@+  
       .@@@+@@@ 
       @@     @@     Cyclone Scheme->C compiler
      ,@             http://justinethier.github.io/cyclone/
      '@        
      .@             (c) 2014-2020 Justin Ethier
       @@     #@     Version 0.19 
       `@@@#@@@.
        #@@@@@   
        +@@@+    
        @@#      
      `@.  

cyclone> (import (cyclone bytevector))
ok
cyclone> (define bv (make-bytevector 8 0))
ok
cyclone> (bytevector-ieee-double-native-set! bv 0 1.0)
ld-elf.so.1: /usr/local/share/cyclone/srfi/60.so: Undefined symbol "mp_and"
justinethier commented 4 years ago

Hi @arthurmaciel

I reinstalled cyclone-winds and did sudo cyclone-winds reinstall bytevector.

After doing that I got this result:

cyclone> (import (cyclone bytevector))
ok
cyclone> (define bv (make-bytevector 8 0))
ok
cyclone> (bytevector-ieee-double-native-set! bv 0 1.0)
#u8(0 0 0 128 1 1 241 63)