Chris00 / ocaml-cairo

Binding to Cairo, a 2D Vector Graphics Library.
GNU Lesser General Public License v3.0
54 stars 8 forks source link

Segfault in image_create test #19

Closed jamesjer closed 3 years ago

jamesjer commented 4 years ago

Building ocaml-cairo 0.6.1 with ocaml 4.10.0 beta1 on a Fedora Rawhide machine, tests/image_create.ml segfaults. Running with valgrind shows that the data array allocated on line 6 is deallocated in one of the Gc.compact() calls on line 17, leading to a segfault as cr is used in the next few lines. The build logs can be seen here: https://koji.fedoraproject.org/koji/taskinfo?taskID=40896397.

The problem appears to be that the result of create() is an opaque object, so OCaml finds no remaining references to data, allowing the garbage collector to reclaim it, even though cr still contains a pointer into its memory. Is this a problem with the test only, or does this imply that the interface should be changed to ensure that a reference to the array lives as long as the Surface object?

wizeman commented 4 years ago

I think I'm running into the same problem on NixOS, as the mentioned test seems to be getting a segfault.

Build log for reference:

configuring
no configure script, doing nothing
building
      ocamlc src/cairo_stubs.o
In file included from cairo_stubs.c:35:
cairo_ocaml_types.h: In function 'caml_cairo_raise_Error':
cairo_ocaml_types.h:53:11: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
   53 |       exn = caml_named_value("Cairo.Error");
      |           ^
In file included from cairo_stubs.c:24:
cairo_stubs.c: In function 'caml_cairo_output_string':
/nix/store/s15p67638bwhm78rf1z13p9zh9kmsdj1-ocaml-4.10.0/lib/ocaml/caml/mlvalues.h:265:24: warning: passing argument 1 of 'memmove' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  265 | #define String_val(x) ((const char *) Bp_val(x))
      |                       ~^~~~~~~~~~~~~~~~~~~~~~~~~
cairo_stubs.c:1684:11: note: in expansion of macro 'String_val'
 1684 |   memmove(String_val(s), data, length);
      |           ^~~~~~~~~~
In file included from /nix/store/c9l946swbx2v67mwamd3vrypha4l9nz0-glibc-2.30-dev/include/features.h:450,
                 from /nix/store/c9l946swbx2v67mwamd3vrypha4l9nz0-glibc-2.30-dev/include/bits/libc-header-start.h:33,
                 from /nix/store/c9l946swbx2v67mwamd3vrypha4l9nz0-glibc-2.30-dev/include/string.h:26,
                 from cairo_stubs.c:18:
/nix/store/c9l946swbx2v67mwamd3vrypha4l9nz0-glibc-2.30-dev/include/bits/string_fortified.h:38:1: note: expected 'void *' but argument is of type 'const char *'
   38 | __NTH (memmove (void *__dest, const void *__src, size_t __len))
      | ^~~~~
running tests
image_create alias tests/runtest (got signal SEGV)
(cd _build/default/tests && ./image_create.exe)
DESTROY bigarray 'data'
Fontconfig error: Cannot load default config file

builder for '/nix/store/fswb8dmfzdjiyb276a72xkf2gi8lh33i-ocaml4.10.0-cairo2-0.6.1.drv' failed with exit code 1
Chris00 commented 3 years ago

@jamesjer Well, the caml_cairo_image_bigarray_attach_proxy(surf, b) in caml_cairo_image_surface_create_for_data8 is supposed to care for that dependency. I'll investigate.

Chris00 commented 3 years ago

A fix has been pushed to the master branch. May you please report whether it works for you?