It looks like strerror should continue to be used on Windows, as it returns a pointer to a thread-local buffer.

Also linking in #10505, as the POSIX 2001 part of that might therefore be relevant for 5.00.0

Thanks for tackling strerror.

• We pepper around _GNU_SOURCE at various points, so it unnerves me that there's a reliance on the definition of strerror_r that we get in a header. That said, it looks like the whole of sync.h should be guarded with CAML_INTERNALS anyway, which would lessen that worry.

How about defining a caml_safe_strerror in a minimal C file where you have more tight control over headers and macros, and then using that?
Something like (might need some ifdef WIN32 here or in a header to just use strerror with the per-thread buffer):

#undef _GNU_SOURCE

#include <string.h>
#if _POSIX_C_SOURCE < 200112L
#error "string.h is too old"
#endif

#include <errno.h>
#include <stdio.h>

void caml_safe_strerror(int errnum, char* strerrbuf, size_t buflen)
{
    int rc;
    if (!strerrbuf) return;
    if ((rc = strerror_r(errnum, strerrbuf, buflen))) {
        int err = rc == -1 ? errno : rc;
        snprintf(strerrbuf, buflen, "errno (original: %d, strerror_r: %d)", errnum, err);
    }
}

Some advantages:

• it is a void function that always succeeds (falling back to just printing errno in case strerror_r fails for whatever reason
• it doesn't crash if strerrbuf is NULL (although this should never happen, one could sprinkle some GCC attributes in the header defining caml_strerror to warn if strerrbuf is NULL or buflen is not the right size
• it avoids some header potentially defining _GNU_SOURCE and getting the wrong function here (or perhaps some defining _GNU_SOURCE in CFLAGS for some reason)

Disadvantage:

• an extra function call when errors are encountered (though OCaml will likely end up raising an exception in this case anyway, so likely not on a performance critical path)
• it doesn't handle failures from snprintf itself (should it just strncpy a static string in that case?)

It is very unlikely that strerror_r itself would fail in practice, so maybe the fallback code could just print errno %d?

Many thanks for the rapid comments, @dra27 @edwintorok. I do prefer the idea of a caml_safe_strerror function rather than peppering the source with ERANGE checks.

One thought I had to simplify the caml_safe_strerror function is that we could give it a __thread local buffer to copy the results into. This would allow us to use the GNU variant (which might return a pointer to some other buffer) and also the Windows strerror would just work. I think this is safe since the only thing the runtime does with the strerror result is to blit it into some OCaml value right after.

I like the idea of a thread local buffer, but if you use the GNU variant we should have some fallback code for non-Linux and non-Windows OSes (I'm assuming the POSIX strerror_r is the only thing available there?)

It looks like strerror should continue to be used on Windows, as it returns a pointer to a thread-local buffer.

I'm pretty sure many C libraries implement strerror like that too, but nobody wants to say it in the docs.

Re caml_fatal_error_arg_2, #10966 already proposes to get rid of it.

This is ready for re-review now. It implements a caml_strerror which use a TLS buffer and doesn't do any unnecessary copying now (hopefully). Tested on Linux glibc, FreeBSD, macOS. Still need to do OpenBSD and Linux musl.

Yes, of course. It is unsatisfying to have to cart around 1K of extra TLS, especially when most of the time the underlying implementation returns a pointer to a statically declared string (but might not).

Regarding __thread, we'll have to fix the use of that in a number of other places in the runtime. My understanding of the more standard thread_local in C11 is that it comes with a potential runtime performance penalty due to differing static initialisation semantics from __thread.

I've been peering inside the implementation of strerror functions, and the only real case where corruption might result seems to be if another thread changes the locale while an strerror() is running and so the returned pointers to error strings will mutate. I guess we can't control the case where we're embedded in a C library and that changes the locale, so I'm discounting this line of thinking unless someone else can spot a way.

Apologies for leaving you hanging. My trivial variation on this PR is here: #11010.