From fd0cb184315e3ca7b0ba8c9dcba2d3a331447503 Mon Sep 17 00:00:00 2001 From: Simmo Saan Date: Wed, 28 Feb 2024 10:39:31 +0200 Subject: [PATCH] Document intentional GC rule violations in floating-point C stubs (closes #1371) --- src/common/cdomains/floatOps/floatOps.ml | 1 + src/common/cdomains/floatOps/stubs.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/common/cdomains/floatOps/floatOps.ml b/src/common/cdomains/floatOps/floatOps.ml index a4e39d930e..a791778857 100644 --- a/src/common/cdomains/floatOps/floatOps.ml +++ b/src/common/cdomains/floatOps/floatOps.ml @@ -1,3 +1,4 @@ +(* Order must match with round_mode in stubs.c *) type round_mode = | Nearest | ToZero diff --git a/src/common/cdomains/floatOps/stubs.c b/src/common/cdomains/floatOps/stubs.c index 50e4a2fb31..b98b26c1da 100644 --- a/src/common/cdomains/floatOps/stubs.c +++ b/src/common/cdomains/floatOps/stubs.c @@ -6,6 +6,7 @@ #include #include +// Order must match with round_mode in floatOps.ml enum round_mode { Nearest, @@ -39,12 +40,16 @@ static void change_round_mode(int mode) #define UNARY_OP(name, type, op) \ CAMLprim value name##_##type(value mode, value x) \ { \ + /* No need to use CAMLparam to keep mode and x as GC roots, + because next GC poll point is at allocation in caml_copy_double. + We have already read their values by then. */ \ 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); \ + /* No need to use CAMLreturn because we don't use CAMLparam. */ \ } UNARY_OP(sqrt, double, sqrt); @@ -53,12 +58,16 @@ UNARY_OP(sqrt, float, sqrtf); #define BINARY_OP(name, type, op) \ CAMLprim value name##_##type(value mode, value x, value y) \ { \ + /* No need to use CAMLparam to keep mode, x and y as GC roots, + because next GC poll point is at allocation in caml_copy_double. + We have already read their values by then. */ \ int old_roundingmode = fegetround(); \ change_round_mode(Int_val(mode)); \ volatile type r, x1 = Double_val(x), y1 = Double_val(y); \ r = x1 op y1; \ fesetround(old_roundingmode); \ return caml_copy_double(r); \ + /* No need to use CAMLreturn because we don't use CAMLparam. */ \ } BINARY_OP(add, double, +); @@ -72,6 +81,9 @@ BINARY_OP(div, float, /); CAMLprim value atof_double(value mode, value str) { + // No need to use CAMLparam to keep mode and str as GC roots, + // because next GC poll point is at allocation in caml_copy_double. + // We have already read their values by then. const char *s = String_val(str); volatile double r; int old_roundingmode = fegetround(); @@ -79,10 +91,14 @@ CAMLprim value atof_double(value mode, value str) r = atof(s); fesetround(old_roundingmode); return caml_copy_double(r); + // No need to use CAMLreturn because we don't use CAMLparam. } CAMLprim value atof_float(value mode, value str) { + // No need to use CAMLparam to keep mode and str as GC roots, + // because next GC poll point is at allocation in caml_copy_double. + // We have already read their values by then. const char *s = String_val(str); volatile float r; int old_roundingmode = fegetround(); @@ -90,14 +106,21 @@ CAMLprim value atof_float(value mode, value str) r = (float)atof(s); fesetround(old_roundingmode); return caml_copy_double(r); + // No need to use CAMLreturn because we don't use CAMLparam. } CAMLprim value max_float(value unit) { + // No need to use CAMLparam to keep unit as GC root, + // because we don't use it. return caml_copy_double(FLT_MAX); + // No need to use CAMLreturn because we don't use CAMLparam. } CAMLprim value smallest_float(value unit) { + // No need to use CAMLparam to keep unit as GC root, + // because we don't use it. return caml_copy_double(FLT_MIN); + // No need to use CAMLreturn because we don't use CAMLparam. }