You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Original bug ID: 7468 Reporter:@oandrieu Assigned to:@xavierleroy Status: resolved (set by @xavierleroy on 2017-02-16T19:01:55Z) Resolution: fixed Priority: normal Severity: minor Version: 4.04.0 Target version: 4.06.0 +dev/beta1/beta2/rc1 Fixed in version: 4.06.0 +dev/beta1/beta2/rc1 Category: runtime system and C interface
Bug description
The helper function caml_alloc_sprintf works by calling vsnprintf, then caml_alloc_string, then it may call vsnprintf again if the result does not fit in the 64 bytes static buffer.
This means that it's unsafe to use an OCaml string via String_val as the format string or as an argument since the caml_alloc_string call may trigger a GC and move the string around and leave caml_alloc_sprintf with a stale pointer.
In floats.c, caml_format_float calls caml_alloc_sprintf in such a way:
res = caml_alloc_sprintf(String_val(fmt), d);
and in CamlInternalFormat, format_float is called with a constructed OCaml string which may be moved or reclaimed by the GC.
Additional information
I can provide a patch if you agree on the analysis.
The text was updated successfully, but these errors were encountered:
I was thinking of adding an caml_alloc_sprintf variant with an extra indirection for the format string, and calling it in caml_format_float with a local C root: oandrieu@6b56448
(untested)
It's a bit complicated because of the variadic function, and now I realize that va_copy is C99, which means it's not available for MSVC < 2013 ...
Another option is to simply strdup() the OCaml string in caml_format_float.
Thanks for the sample code. The alternative I was considering is to caml_strdup() the format in caml_alloc_sprintf() before caml_alloc_string() is called, but only in the infrequent case we need to redo the snprintf.
What makes me grumpy is that I can't write a repro case for the bug, even by declaring caml_format_float as an external and calling it directly in the test with a format argument that resides in the minor heap.
Original bug ID: 7468
Reporter: @oandrieu
Assigned to: @xavierleroy
Status: resolved (set by @xavierleroy on 2017-02-16T19:01:55Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.04.0
Target version: 4.06.0 +dev/beta1/beta2/rc1
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: runtime system and C interface
Bug description
The helper function caml_alloc_sprintf works by calling vsnprintf, then caml_alloc_string, then it may call vsnprintf again if the result does not fit in the 64 bytes static buffer.
This means that it's unsafe to use an OCaml string via String_val as the format string or as an argument since the caml_alloc_string call may trigger a GC and move the string around and leave caml_alloc_sprintf with a stale pointer.
In floats.c, caml_format_float calls caml_alloc_sprintf in such a way:
res = caml_alloc_sprintf(String_val(fmt), d);
and in CamlInternalFormat, format_float is called with a constructed OCaml string which may be moved or reclaimed by the GC.
Additional information
I can provide a patch if you agree on the analysis.
The text was updated successfully, but these errors were encountered: