[Caml-list] memory corruption with Weak.t in Gc.finalise

[Caml-list] memory corruption with Weak.t in Gc.finalise

[Goswin von Brederlow]

Hi,

I found a memory corruption in Gc.finalise when sharing a Waek.t with
C stubs. I made a small test case and it's gotten worse. The code now
segfaults:

  https://github.com/mrvn/ocam-problems/tree/weak-finalise

The problem I had was the address of the obj stored in the Weak.t
changes (which might be the GC moving it?) and also the tag changes
from 248 (Object_tag) to 168 (random constructor).

With my smaller test case I further get:

Program received signal SIGSEGV, Segmentation fault.
0x000000000043c9f8 in caml_format_exception ()
(gdb) bt
#0  0x000000000043c9f8 in caml_format_exception ()
#1  0x000000000043cbc1 in caml_fatal_uncaught_exception ()
#2  0x000000000043b1a7 in caml_main ()
#3  0x000000000042c45d in main ()

Ideas what is going on there?

MfG
	Goswin

------------------------------------------------------------------------
==> Makefile <==
all: test
        ./test

test: main.cmx stubs.o
        ocamlopt -o $@ $+

%.cmx: %.ml
        ocamlopt -g -c -o $@ $<

%.o: %.c
        gcc -O2 -W -Wall -g -c -o $@ $<

clean:
        rm -f test *.o *.cmx *.cmi *~

==> main.ml <==
external setup : 'a Weak.t -> unit = "setup"
external test : unit -> unit = "test"

let cleanup obj =
  Printf.printf "cleanup\n%!";
  test ()

class obj = object(self)
  val weak = Weak.create 1
  initializer Weak.set weak 0 (Some self)
  initializer setup weak
  initializer Gc.finalise cleanup self
end

let () =
  Printf.printf "Creating object\n%!";
  ignore (new obj);
  test ()

let () =
  Printf.printf "Garbage collection\n%!";
  Gc.full_major ()

let () =
  Printf.printf "checking after GC\n%!";
  test ()

==> stubs.c <==
#include <stdio.h>
#include <caml/mlvalues.h>
#include <caml/memory.h>

value weak;

void setup(value ml_weak) {
    CAMLparam1(ml_weak);
    printf("%s(0x%lx)\n", __PRETTY_FUNCTION__, ml_weak);
    weak = ml_weak;
    caml_register_generational_global_root(&weak);
    CAMLreturn0;
}

void test(void) {
    CAMLparam0();
    CAMLlocal1(obj);
    printf("%s\n", __PRETTY_FUNCTION__);
    printf("  weak = 0x%lx\n", weak);
    if (weak != 0) {
        obj = Field(weak, 1);
        printf("  obj = 0x%lx\n", obj);
        if (obj != 0) {
            printf("  tag = %d\n", Tag_val(obj));
        }
    }
    printf("%s done\n", __PRETTY_FUNCTION__);
    CAMLreturn0;
}

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[Gabriel Scherer]

It's great that you have a minimal reproduction example. Could you
create an issue on the issue tracker?
  http://caml.inria.fr/mantis/

Issues on the bugtracker are easier to keep track of than mails on the
caml-list.


On Tue, Mar 8, 2016 at 10:49 AM, Goswin von Brederlow <goswin-v-b@web.de> wrote:
> Hi,
>
> I found a memory corruption in Gc.finalise when sharing a Waek.t with
> C stubs. I made a small test case and it's gotten worse. The code now
> segfaults:
>
>   https://github.com/mrvn/ocam-problems/tree/weak-finalise
>
> The problem I had was the address of the obj stored in the Weak.t
> changes (which might be the GC moving it?) and also the tag changes
> from 248 (Object_tag) to 168 (random constructor).
>
> With my smaller test case I further get:
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000000000043c9f8 in caml_format_exception ()
> (gdb) bt
> #0  0x000000000043c9f8 in caml_format_exception ()
> #1  0x000000000043cbc1 in caml_fatal_uncaught_exception ()
> #2  0x000000000043b1a7 in caml_main ()
> #3  0x000000000042c45d in main ()
>
> Ideas what is going on there?
>
> MfG
>         Goswin
>
> ------------------------------------------------------------------------
> ==> Makefile <==
> all: test
>         ./test
>
> test: main.cmx stubs.o
>         ocamlopt -o $@ $+
>
> %.cmx: %.ml
>         ocamlopt -g -c -o $@ $<
>
> %.o: %.c
>         gcc -O2 -W -Wall -g -c -o $@ $<
>
> clean:
>         rm -f test *.o *.cmx *.cmi *~
>
> ==> main.ml <==
> external setup : 'a Weak.t -> unit = "setup"
> external test : unit -> unit = "test"
>
> let cleanup obj =
>   Printf.printf "cleanup\n%!";
>   test ()
>
> class obj = object(self)
>   val weak = Weak.create 1
>   initializer Weak.set weak 0 (Some self)
>   initializer setup weak
>   initializer Gc.finalise cleanup self
> end
>
> let () =
>   Printf.printf "Creating object\n%!";
>   ignore (new obj);
>   test ()
>
> let () =
>   Printf.printf "Garbage collection\n%!";
>   Gc.full_major ()
>
> let () =
>   Printf.printf "checking after GC\n%!";
>   test ()
>
> ==> stubs.c <==
> #include <stdio.h>
> #include <caml/mlvalues.h>
> #include <caml/memory.h>
>
> value weak;
>
> void setup(value ml_weak) {
>     CAMLparam1(ml_weak);
>     printf("%s(0x%lx)\n", __PRETTY_FUNCTION__, ml_weak);
>     weak = ml_weak;
>     caml_register_generational_global_root(&weak);
>     CAMLreturn0;
> }
>
> void test(void) {
>     CAMLparam0();
>     CAMLlocal1(obj);
>     printf("%s\n", __PRETTY_FUNCTION__);
>     printf("  weak = 0x%lx\n", weak);
>     if (weak != 0) {
>         obj = Field(weak, 1);
>         printf("  obj = 0x%lx\n", obj);
>         if (obj != 0) {
>             printf("  tag = %d\n", Tag_val(obj));
>         }
>     }
>     printf("%s done\n", __PRETTY_FUNCTION__);
>     CAMLreturn0;
> }
>
> --
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[Goswin von Brederlow]

On Tue, Mar 08, 2016 at 11:00:33AM -0500, Gabriel Scherer wrote:
> It's great that you have a minimal reproduction example. Could you
> create an issue on the issue tracker?
>   http://caml.inria.fr/mantis/
> 
> Issues on the bugtracker are easier to keep track of than mails on the
> caml-list.

If it is a bug in the compiler and not just my code then sure.

MfG
	Goswin
 
> On Tue, Mar 8, 2016 at 10:49 AM, Goswin von Brederlow <goswin-v-b@web.de> wrote:
> > Hi,
> >
> > I found a memory corruption in Gc.finalise when sharing a Waek.t with
> > C stubs. I made a small test case and it's gotten worse. The code now
> > segfaults:
> >
> >   https://github.com/mrvn/ocam-problems/tree/weak-finalise
> >
> > The problem I had was the address of the obj stored in the Weak.t
> > changes (which might be the GC moving it?) and also the tag changes
> > from 248 (Object_tag) to 168 (random constructor).
> >
> > With my smaller test case I further get:
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x000000000043c9f8 in caml_format_exception ()
> > (gdb) bt
> > #0  0x000000000043c9f8 in caml_format_exception ()
> > #1  0x000000000043cbc1 in caml_fatal_uncaught_exception ()
> > #2  0x000000000043b1a7 in caml_main ()
> > #3  0x000000000042c45d in main ()
> >
> > Ideas what is going on there?
> >
> > MfG
> >         Goswin
> >
> > ------------------------------------------------------------------------
> > ==> Makefile <==
> > all: test
> >         ./test
> >
> > test: main.cmx stubs.o
> >         ocamlopt -o $@ $+
> >
> > %.cmx: %.ml
> >         ocamlopt -g -c -o $@ $<
> >
> > %.o: %.c
> >         gcc -O2 -W -Wall -g -c -o $@ $<
> >
> > clean:
> >         rm -f test *.o *.cmx *.cmi *~
> >
> > ==> main.ml <==
> > external setup : 'a Weak.t -> unit = "setup"
> > external test : unit -> unit = "test"
> >
> > let cleanup obj =
> >   Printf.printf "cleanup\n%!";
> >   test ()
> >
> > class obj = object(self)
> >   val weak = Weak.create 1
> >   initializer Weak.set weak 0 (Some self)
> >   initializer setup weak
> >   initializer Gc.finalise cleanup self
> > end
> >
> > let () =
> >   Printf.printf "Creating object\n%!";
> >   ignore (new obj);
> >   test ()
> >
> > let () =
> >   Printf.printf "Garbage collection\n%!";
> >   Gc.full_major ()
> >
> > let () =
> >   Printf.printf "checking after GC\n%!";
> >   test ()
> >
> > ==> stubs.c <==
> > #include <stdio.h>
> > #include <caml/mlvalues.h>
> > #include <caml/memory.h>
> >
> > value weak;
> >
> > void setup(value ml_weak) {
> >     CAMLparam1(ml_weak);
> >     printf("%s(0x%lx)\n", __PRETTY_FUNCTION__, ml_weak);
> >     weak = ml_weak;
> >     caml_register_generational_global_root(&weak);
> >     CAMLreturn0;
> > }
> >
> > void test(void) {
> >     CAMLparam0();
> >     CAMLlocal1(obj);
> >     printf("%s\n", __PRETTY_FUNCTION__);
> >     printf("  weak = 0x%lx\n", weak);
> >     if (weak != 0) {
> >         obj = Field(weak, 1);
> >         printf("  obj = 0x%lx\n", obj);
> >         if (obj != 0) {
> >             printf("  tag = %d\n", Tag_val(obj));
> >         }
> >     }
> >     printf("%s done\n", __PRETTY_FUNCTION__);
> >     CAMLreturn0;
> > }
> >
> > --
> > Caml-list mailing list.  Subscription management and archives:
> > https://sympa.inria.fr/sympa/arc/caml-list
> > Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> > Bug reports: http://caml.inria.fr/bin/caml-bugs
> 
> -- 
> Caml-list mailing list.  Subscription management and archives:
> https://sympa.inria.fr/sympa/arc/caml-list
> Beginner's list: http://groups.yahoo.com/group/ocaml_beginners
> Bug reports: http://caml.inria.fr/bin/caml-bugs

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[François Bobot]

On 08/03/2016 20:04, Goswin von Brederlow wrote:
> On Tue, Mar 08, 2016 at 11:00:33AM -0500, Gabriel Scherer wrote:
>> It's great that you have a minimal reproduction example. Could you
>> create an issue on the issue tracker?
>>    http://caml.inria.fr/mantis/
>>
>> Issues on the bugtracker are easier to keep track of than mails on the
>> caml-list.
>
> If it is a bug in the compiler and not just my code then sure.
>

Perhaps a bug in your code, I haven't ran your code; just read it.

>>>
>>> class obj = object(self)
>>>    val weak = Weak.create 1
>>>    initializer Weak.set weak 0 (Some self)
>>>    initializer setup weak
>>>    initializer Gc.finalise cleanup self
>>> end
>>>
>>> let () =
>>>    Printf.printf "Creating object\n%!";
>>>    ignore (new obj);
>>>    test ()
>>>

>>> void test(void) {
>>>      CAMLparam0();
>>>      CAMLlocal1(obj);
>>>      printf("%s\n", __PRETTY_FUNCTION__);
>>>      printf("  weak = 0x%lx\n", weak);
>>>      if (weak != 0) {
>>>          obj = Field(weak, 1);
>>>          printf("  obj = 0x%lx\n", obj);
>>>          if (obj != 0) {
>>>              printf("  tag = %d\n", Tag_val(obj));
>>>          }

If the weak pointer is unset, obj is not equal to 0 but to caml_weak_none (in 4.03/trunk 
caml_ephe_none). And I think you are in this case since `(new obj)` can be immediately garbage 
collected. caml_ephe_none should not be dereferenced:
```
static value ephe_dummy = 0;
value caml_ephe_none = (value) &ephe_dummy;
```

Thank you for your mantis ticket http://caml.inria.fr/mantis/view.php?id=7173 that asks for a C API 
for weak pointers/ephemerons.

Best,

-- 
François

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[Goswin von Brederlow]

BOn Fri, Mar 11, 2016 at 04:05:25PM +0100, François Bobot wrote:
> On 08/03/2016 20:04, Goswin von Brederlow wrote:
> >On Tue, Mar 08, 2016 at 11:00:33AM -0500, Gabriel Scherer wrote:
> >>It's great that you have a minimal reproduction example. Could you
> >>create an issue on the issue tracker?
> >>   http://caml.inria.fr/mantis/
> >>
> >>Issues on the bugtracker are easier to keep track of than mails on the
> >>caml-list.
> >
> >If it is a bug in the compiler and not just my code then sure.
> >
> 
> Perhaps a bug in your code, I haven't ran your code; just read it.
> 
> >>>
> >>>class obj = object(self)
> >>>   val weak = Weak.create 1
> >>>   initializer Weak.set weak 0 (Some self)
> >>>   initializer setup weak
> >>>   initializer Gc.finalise cleanup self
> >>>end
> >>>
> >>>let () =
> >>>   Printf.printf "Creating object\n%!";
> >>>   ignore (new obj);
> >>>   test ()
> >>>
> 
> >>>void test(void) {
> >>>     CAMLparam0();
> >>>     CAMLlocal1(obj);
> >>>     printf("%s\n", __PRETTY_FUNCTION__);
> >>>     printf("  weak = 0x%lx\n", weak);
> >>>     if (weak != 0) {
> >>>         obj = Field(weak, 1);
> >>>         printf("  obj = 0x%lx\n", obj);
> >>>         if (obj != 0) {
> >>>             printf("  tag = %d\n", Tag_val(obj));
> >>>         }
> 
> If the weak pointer is unset, obj is not equal to 0 but to caml_weak_none
> (in 4.03/trunk caml_ephe_none). And I think you are in this case since `(new
> obj)` can be immediately garbage collected. caml_ephe_none should not be
> dereferenced:
> ```
> static value ephe_dummy = 0;
> value caml_ephe_none = (value) &ephe_dummy;
> ```
> 
> Thank you for your mantis ticket
> http://caml.inria.fr/mantis/view.php?id=7173 that asks for a C API for weak
> pointers/ephemerons.
> 
> Best,

Still using 4.02. This is a new thing.

Still Weak.t needs to interact cleanly with the Gc. Guessing at the
internal state of Weak.t is bad. Let's fix this for 4.03.

MfG
	Goswin

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[François Bobot]

On 14/03/2016 11:32, Goswin von Brederlow wrote:
>
> Still using 4.02. This is a new thing.
>
> Still Weak.t needs to interact cleanly with the Gc. Guessing at the
> internal state of Weak.t is bad. Let's fix this for 4.03.

I think it is already fixed for 4.03. The new GC phase named "clean", between mark and sweep, makes 
the behavior of weak pointers straightforward compared to previous OCaml version:
  - A weak pointer is unset only if the value pointed is marked dead after the mark phase (ie. no 
way to ever access it, the memory could be freed).
  - Conversely, during the clean phase, a weak pointer to a dead value is considered unset (so 
Weak.get return None).

If you know weird behavior in 4.03+beta on weak pointers or ephemeron, I would be very interested to 
know them.

Best,

-- 
François

Re: [Caml-list] memory corruption with Weak.t in Gc.finalise

[Goswin von Brederlow]

On Mon, Mar 14, 2016 at 05:44:28PM +0100, François Bobot wrote:
> On 14/03/2016 11:32, Goswin von Brederlow wrote:
> >
> >Still using 4.02. This is a new thing.
> >
> >Still Weak.t needs to interact cleanly with the Gc. Guessing at the
> >internal state of Weak.t is bad. Let's fix this for 4.03.
> 
> I think it is already fixed for 4.03. The new GC phase named "clean",
> between mark and sweep, makes the behavior of weak pointers straightforward
> compared to previous OCaml version:
>  - A weak pointer is unset only if the value pointed is marked dead after
> the mark phase (ie. no way to ever access it, the memory could be freed).
>  - Conversely, during the clean phase, a weak pointer to a dead value is
> considered unset (so Weak.get return None).
> 
> If you know weird behavior in 4.03+beta on weak pointers or ephemeron, I
> would be very interested to know them.
> 
> Best,

I heard about that design cleanup. But what I ment was to provide and
document an interface for C code interacting with Weak.t.

MfG
	Goswin