jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

Crash on C functions that may return null pointers #61

Closed hugopl closed 4 years ago

hugopl commented 4 years ago

Not sure if this is caused by wrong GIR annotations on GtkSourceView [1], but the generated bindings for GtkSource::StyleSchemeManager#scheme is not returing nil when the C method returns a null pointer.

    def scheme(scheme_id : ::String)
      __return_value = LibGtkSource.style_scheme_manager_get_scheme(@pointer.as(LibGtkSource::StyleSchemeManager*), scheme_id.to_unsafe)
      GtkSource::StyleScheme.new(__return_value)
    end

So the following code crash:

require "gobject/gtk/autorun"
require_gobject "GtkSource"

GtkSource.init
manager = GtkSource::StyleSchemeManager.default
puts manager.to_unsafe
manager.scheme("any wrong id")

Backtrace

Pointer(LibGtkSource::StyleSchemeManager)@0x55fa564ff0e0
Invalid memory access (signal 11) at address 0x0
[0x55fa548e37f6] *CallStack::print_backtrace:Int32 +118
[0x55fa548d62ce] __crystal_sigfault_handler +286
[0x7f4d0efa5800] ???
[0x55fa5493a294] *GtkSource::StyleScheme +4
[0x55fa5493a286] *GtkSource::StyleScheme +6
[0x55fa5493a253] *GtkSource::StyleScheme#initialize<Pointer(LibGtkSource::StyleScheme)>:Nil +67
[0x55fa5493a1f8] *GtkSource::StyleScheme::new<Pointer(LibGtkSource::StyleScheme)>:GtkSource::StyleScheme +104
[0x55fa54939fe2] *GtkSource::StyleSchemeManager#scheme<String>:GtkSource::StyleScheme +66
[0x55fa548c1ae5] __crystal_main +1157
[0x55fa5493ad86] *Crystal::main_user_code<Int32, Pointer(Pointer(UInt8))>:Nil +6
[0x55fa548d633c] main +60
[0x7f4d0ed71023] __libc_start_main +243
[0x55fa548c158e] _start +46
[0x0] ???

[1] https://developer.gnome.org/gtksourceview/stable/GtkSourceStyleSchemeManager.html#gtk-source-style-scheme-manager-get-scheme

jhass commented 4 years ago

Yeah, the typelib is just:

      + get_scheme (function, GtkSource)
        * method = true
        * throws = false
        * skip_return = false
        * may_return_null = false
        * caller_owns = NOTHING
        * self_arg_ownership = NOTHING
        * return_type
            + <no name> (type, GtkSource)
              * tag = INTERFACE
              * pointer = true
              * interface
                + StyleScheme (object, GtkSource)

So may_return_null = false. We could in theory ignore this flag and always assume it can return a null pointer, but I found it better to just maintain manual overrides for these cases (and perhaps try to report it upstream).

Funnily enough GIRepository itself has the same issue in a couple of places, hence https://github.com/jhass/crystal-gobject/blob/8d87bb7b0d87534b2ce78c3add558bd554bf6fbe/src/g_i_repository/repository.cr#L38-L42

jhass commented 4 years ago

Though maybe we can improve the error message by asserting a non null pointer, potentially even suggesting to add a manual override. What do you think?

jhass commented 4 years ago

Alright, I implemented the above idea :)

hugopl commented 4 years ago

Yes, IMO assert the null pointer is better, since the bug is on C library in this case, not on Crystal bindings.

hugopl commented 4 years ago

Issue filled upstream https://gitlab.gnome.org/GNOME/gtksourceview/-/issues/133