[Caml-list] Initializing CAMLlocalX values to Val_unit

[Caml-list] Initializing CAMLlocalX values to Val_unit

[Richard W.M. Jones]

We just corrected a crashing bug in some C bindings.  The code
essentially did:

  CAMLlocal1 (exn);

  call_some_function (&exn);

  if (exn != Val_unit) {
    // The function wants to raise an exception ...
    caml_raise (exn);
  }

This crashed with OCaml 4.01 because of Mark's upstream change to how
CAMLlocalX is initialized.

https://github.com/ocaml/ocaml/commit/05100e597e4296a2e79e6c2d9cd75b7e1cc595c9
https://github.com/ocaml/ocaml/commit/2dd92969d254ddae7b49f1478a2ef69ccf70ad42

In OCaml <= 4.01, there was an implicit initialization to 0 in the
macro.  In some later version of OCaml this was changed to implicitly
initialize to Val_unit instead.

I have no problem with this change, and the code above was obviously
buggy with the older versions of OCaml.  However I do have some
questions ...

(1) The documentation says:

  "The macros CAMLlocal1 to CAMLlocal5 declare and initialize one to
  five local variables"

which is technically correct, but not very useful.  Is the
initialization with Val_unit a permanent change, and will it be
documented as ABI?

(2) We have lots of existing code which does:

  CAMLlocal1 (v);
  // some code here which might allocate and call the GC
  v = ...

It's my understanding that such code might crash with the older
versions of OCaml, because when it calls the GC it will find a local
root which is initialized to 0.  We want to continue to support the
older versions (back to 3.12 in fact), so should we change it all to:

  CAMLlocal1 (v); v = Val_unit;
  // some code here which might allocate and call the GC
  v = ...

?  Interestingly none of this code has actually crashed in production
as far as I'm aware.

Rich.

Re: [Caml-list] Initializing CAMLlocalX values to Val_unit

[Arlen Cox]

> ?  Interestingly none of this code has actually crashed in production
> as far as I'm aware.

I'm fairly certain this is because the OCaml runtime checks pointers
to ensure that they're in the OCaml heap before tracing them.  I don't
know how this holds up historically, but last time I looked (4.03)
when tracing, OCaml runs Classify_addr(a) on addresses to determine if
they should be traced into.  Because of this, NULL pointers will not
be followed (and thus not crash) and perhaps more interestingly raw
(naked) pointers that don't point to anything in an OCaml-registered
page will also not be traced.

There is a define related to NAKED_POINTERS that if defined will
improve performance by disabling the check for Is_in_heap(a) (macro
for Classify_addr).  Naked pointer support can be disabled during
configure (--no-naked-pointers) and that may make code that does not
initialize things correctly crash.  Naked pointers are checked in the
default configuration.

Cheers,
Arlen

>
> Rich.
>
> --
> 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] Initializing CAMLlocalX values to Val_unit

[Richard W.M. Jones]

On Fri, Feb 24, 2017 at 08:43:38AM -0500, Arlen Cox wrote:
> > ?  Interestingly none of this code has actually crashed in production
> > as far as I'm aware.
> 
> I'm fairly certain this is because the OCaml runtime checks pointers
> to ensure that they're in the OCaml heap before tracing them.

OK I see - I knew this but didn't know if it applied to
NULL pointers too.

> I don't
> know how this holds up historically, but last time I looked (4.03)
> when tracing, OCaml runs Classify_addr(a) on addresses to determine if
> they should be traced into.  Because of this, NULL pointers will not
> be followed (and thus not crash) and perhaps more interestingly raw
> (naked) pointers that don't point to anything in an OCaml-registered
> page will also not be traced.
> 
> There is a define related to NAKED_POINTERS that if defined will
> improve performance by disabling the check for Is_in_heap(a) (macro
> for Classify_addr).  Naked pointer support can be disabled during
> configure (--no-naked-pointers) and that may make code that does not
> initialize things correctly crash.  Naked pointers are checked in the
> default configuration.

Thanks (and also to Gerd's response).

Rich.

-- 
Richard Jones

Re: [Caml-list] Initializing CAMLlocalX values to Val_unit

[Gerd Stolpmann]

[-- Attachment #1: Type: text/plain, Size: 1548 bytes --]

Am Freitag, den 24.02.2017, 10:38 +0000 schrieb Richard W.M. Jones:
> (2) We have lots of existing code which does:
> 
>   CAMLlocal1 (v);
>   // some code here which might allocate and call the GC
>   v = ...
> 
> It's my understanding that such code might crash with the older
> versions of OCaml, because when it calls the GC it will find a local
> root which is initialized to 0.  We want to continue to support the
> older versions (back to 3.12 in fact), so should we change it all to:
> 
>   CAMLlocal1 (v); v = Val_unit;
>   // some code here which might allocate and call the GC
>   v = ...
> 
> ?  Interestingly none of this code has actually crashed in production
> as far as I'm aware.

Because it is correct for older OCaml versions - which do not follow
pointers that are not allocated by OCaml. Note that this is still the
default, but you can now change it with the -no-naked-pointers
configure switch, in which case such pointers lead to a crash when they
are run into by the GC (well, I'm not sure about a null pointers,
though). The -no-naked-pointers switch accelerates in particular
assignments (caml_darken called via caml_modify).

Gerd
-- 
------------------------------------------------------------
Gerd Stolpmann, Darmstadt, Germany    gerd@gerd-stolpmann.de
My OCaml site:          http://www.camlcity.org
Contact details:        http://www.camlcity.org/contact.html
Company homepage:       http://www.gerd-stolpmann.de
------------------------------------------------------------



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]