Exception handling miscompilation on FreeBSD

[dmgk]

When running std_spec, spec/std/gc_spec.cr currently fails with

  1) GC raises if calling enable when not disabled

       GC is not disabled (Exception)
         from src/gc/boehm.cr:174:5 in 'enable'
         from spec/std/gc_spec.cr:10:7 in '->'
         from src/spec/example.cr:45:13 in 'internal_run'
         from src/spec/example.cr:33:16 in 'run'
         from src/spec/context.cr:18:23 in 'internal_run'
         from src/spec/context.cr:339:7 in 'run'
         from src/spec/context.cr:18:23 in 'internal_run'
         from src/spec/context.cr:156:7 in 'run'
         from src/spec/dsl.cr:220:7 in '->'
         from src/crystal/at_exit_handlers.cr:14:19 in 'run'
         from src/crystal/main.cr:50:5 in 'exit'
         from src/crystal/main.cr:45:5 in 'main'
         from src/crystal/main.cr:127:3 in 'main'

It appears that expect_raises for some reason doesn't catch the raised exception: https://github.com/crystal-lang/crystal/blob/6089a210a945563ac29e05b05e6d1033cb5a1c20/spec/std/gc_spec.cr#L8-L12

Looking at this minimal reproducer:

def expect_raises
  yield
rescue ex
  p ex.message
end

def raise_func
  expect_raises do
    GC.enable
  end
end

... it compiles down to the following asm:

        .type   "*raise_func:(String | Nil)",@function
"*raise_func:(String | Nil)":
.Lfunc_begin2217:
        .loc    2 7 0
        .cfi_startproc
        .cfi_personality 155, DW.ref.__crystal_personality
        .cfi_lsda 27, .Lexception51
        pushq   %rax
        .cfi_def_cfa_offset 16
.Ltmp4851:
        .loc    2 10 5 prologue_end
        callq   "*GC::enable:Nil"
        .loc    2 0 5 is_stmt 0
        xorl    %eax, %eax
        .loc    2 2 3 is_stmt 1
        popq    %rcx
        .cfi_def_cfa_offset 8
        retq
.Ltmp4852:
.Lfunc_end2217:
        .size   "*raise_func:(String | Nil)", .Lfunc_end2217-"*raise_func:(String | Nil)"
        .cfi_endproc

It looks like the exception handling was completely omitted. What's interesting is that if I add e.g. p "test" before the condition in GC.enable:

def self.enable
  p "test"                                                                                                                               
  unless LibGC.is_disabled != 0
    raise "GC is not disabled"
  end

  LibGC.enable
end

this makes raise_func to compile with exception handling:

        .type   "*raise_func:(String | Nil)",@function
"*raise_func:(String | Nil)":
.Lfunc_begin2225:
        .loc    2 7 0
        .cfi_startproc
        .cfi_personality 155, DW.ref.__crystal_personality
        .cfi_lsda 27, .Lexception51
        subq    $40, %rsp
        .cfi_def_cfa_offset 48
.Ltmp4867:
.Ltmp4870:
        .loc    2 10 5 prologue_end
        callq   "*GC::enable:Nil"
.Ltmp4868:
        jmp     .LBB2225_3
.LBB2225_2:
.Ltmp4869:
        .loc    2 2 3
        movq    %rax, %rdi
        movl    %edx, 28(%rsp)
        callq   __crystal_get_exception@PLT
        movq    %rax, 16(%rsp)
        jmp     .LBB2225_5
.LBB2225_3:
        .loc    2 0 3 is_stmt 0
        xorl    %eax, %eax
        movl    %eax, %ecx
        movq    %rcx, 8(%rsp)
        .loc    2 10 5 is_stmt 1
        jmp     .LBB2225_4
.LBB2225_4:
        .loc    2 0 5 is_stmt 0
        movq    8(%rsp), %rax
        .loc    2 2 3 is_stmt 1
        addq    $40, %rsp
        .cfi_def_cfa_offset 8
        retq
.LBB2225_5:
        .cfi_def_cfa_offset 48
        .loc    2 0 3 is_stmt 0
        movq    16(%rsp), %rax
        .loc    2 2 3
        movq    %rax, 32(%rsp)
        movq    32(%rsp), %rcx
        movq    8(%rcx), %rdi
        .loc    2 4 3 is_stmt 1
        callq   "*p<(String | Nil)>:(String | Nil)"
        movq    %rax, 8(%rsp)
        jmp     .LBB2225_4
.Ltmp4871:
.Lfunc_end2225:
        .size   "*raise_func:(String | Nil)", .Lfunc_end2225-"*raise_func:(String | Nil)"
        .cfi_endproc

The above is reproducible on FreeBSD 13.1-RELEASE with both llvm10 and llvm14, but not on Arch (LLVM 14.0.6).

[dmgk]

I looked at this some more and it seems that is was inadvertently introduced by https://github.com/crystal-lang/crystal/pull/10259. Calling GC.enable appears to be causing some kind of cycle (GC.enable calls raise, raise uses Exception::CallStack) during the semantic analysis which leads to the def raise() AST node left marked as "non-raising" any exceptions.

I added some tracing to the CleanupTransformer#transform(node : Call) and this is the output on Linux ((true) and (false) is the node's raises? value):

[def self.enable
  if LibGC.is_disabled != 0
  else
    raise("GC is not disabled")
  end
  LibGC.enable
end]
"==========="
[fun is_disabled = GC_is_disabled : Int]
"target_def:  (false) fun is_disabled = GC_is_disabled : Int"
"current_def: (false) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="
[def !=(other : Int32) : Bool
  # primitive: binary
end]
"target_def:  (false) def !=(other : Int32) : Bool\n  # primitive: binary\nend"
"current_def: (false) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="
[def raise(message : String) : NoReturn
  raise(Exception.new(message))
end]
"target_def:  (true) def raise(message : String) : NoReturn\n  raise(Exception.new(message))\nend"
"current_def: (true) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="

def raise(message : String) is marked as raising, and this is propagated to GC.enable. On FreeBSD though, the same code produces:

[def self.enable
  if LibGC.is_disabled != 0
  else
    raise("GC is not disabled")
  end
  LibGC.enable
end]
"==========="
[fun is_disabled = GC_is_disabled : Int]
"target_def:  (false) fun is_disabled = GC_is_disabled : Int"
"current_def: (false) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="
[def !=(other : Int32) : Bool
  # primitive: binary
end]
"target_def:  (false) def !=(other : Int32) : Bool\n  # primitive: binary\nend"
"current_def: (false) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="
[def raise(message : String) : NoReturn
  raise(Exception.new(message))
end]
"target_def:  (false) def raise(message : String) : NoReturn\n  raise(Exception.new(message))\nend"
"current_def: (false) def self.enable\n  if LibGC.is_disabled != 0\n  else\n    raise(\"GC is not disabled\")\n  end\n  LibGC.enable\nend"
"==========="

Here def raise(message : String) node is not marked as raising any exceptions so neither isGC.enable, which gets compiled to the call IR and to the above asm w/o exception handling. Reverting https://github.com/crystal-lang/crystal/pull/10259 fixes this issue on FreeBSD but (possibly) reintroduces https://github.com/crystal-lang/crystal/issues/10084.

I'm not sure what would be the best course of action here. Perhaps GC.enable could just silently return without raising exceptions if GC is already enabled?