Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Floating-point C stubs violate OCaml GC rules #1371

Closed
sim642 opened this issue Feb 26, 2024 · 2 comments
Closed

Floating-point C stubs violate OCaml GC rules #1371

sim642 opened this issue Feb 26, 2024 · 2 comments
Assignees
Labels
bug type-safety Type-safety improvements
Milestone

Comments

@sim642
Copy link
Member

sim642 commented Feb 26, 2024

Our FloatOps C stubs are implemented like

#define UNARY_OP(name, type, op) \
CAMLprim value name##_##type(value mode, value x) \
{ \
int old_roundingmode = fegetround(); \
change_round_mode(Int_val(mode)); \
volatile type r, x1 = Double_val(x); \
r = op(x1); \
fesetround(old_roundingmode); \
return caml_copy_double(r); \
}

These violate GC rules set out by OCaml documentation for C stubs (https://v2.ocaml.org/manual/intfc.html#s%3Ac-gc-harmony):

  1. "A function that has parameters or local variables of type value must begin with a call to one of the CAMLparam macros and return with CAMLreturn, CAMLreturn0, or CAMLreturnT."
  2. "Local variables of type value must be declared with one of the CAMLlocal macros. Arrays of values are declared with CAMLlocalN. These macros must be used at the beginning of the function, not in a nested block."
  3. ...

Is this an oversight or is there some very good reason to not follow the rules?

Not doing so can lead to some very unusual GC bugs: xavierleroy/pringo#6.

@sim642 sim642 added bug type-safety Type-safety improvements labels Feb 26, 2024
@michael-schwarz
Copy link
Member

No, I think it was just an oversight.

@sim642
Copy link
Member Author

sim642 commented Feb 26, 2024

It might actually be fine because caml_copy_double is the only allocating operation in them. Accesses to all value-typed arguments happen before that, so GC won't move/delete anything we want to still access.

OCaml's own floating-point primitives also seem to be similar and not use the macros everywhere: https://github.com/ocaml/ocaml/blob/b2cd7e59e5b563b6a0527014689b7224bb678661/runtime/floats.c.

@sim642 sim642 self-assigned this Feb 26, 2024
@sim642 sim642 closed this as completed in fd0cb18 Feb 28, 2024
@sim642 sim642 added this to the v2.4.0 milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug type-safety Type-safety improvements
Projects
None yet
Development

No branches or pull requests

3 participants