[m-rev.] for review: implement the system RNG on Windows

[Julien Fischer]

Hi Peter,

On Sat, 13 Feb 2021, Peter Wang wrote:

> On Sat, 13 Feb 2021 03:54:24 +1100 Julien Fischer <jfischer at opturion.com> wrote:
>> +#elif defined(ML_SYSRAND_IMPL_RAND_S)
>> +
>> +ML_SystemRandomHandle
>> +ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
>> +{
>> +    *succeeded = MR_TRUE;
>> +    *err_msg = MR_make_string_const(\"\");
>> +    return 0;
>> +}
>> +
>> +MR_Bool
>> +ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
>> +{
>> +    *err_msg = MR_make_string_const(\"\");
>> +    return MR_TRUE;
>> +}
>> +
>> +MR_Bool
>> +ML_random_generate_bytes(ML_SystemRandomHandle handle,
>> +    unsigned char *buffer, size_t len, MR_String *err_msg)
>> +{
>> +    int err;
>> +    unsigned int n;
>> +    unsigned char *bytes = (unsigned char *) &n;
>
> Use a union?

Probably not necessary in the revised version.

>
>> +    size_t num_to_read = len;
>> +    size_t i;
>> +
>> +    while (num_to_read > 0) {
>> +        err = rand_s(&n);
>> +        if (err != 0) {
>> +            *err_msg = MR_make_string_const(\"rand_s failed\");
>> +            return MR_FALSE;
>> +        }
>> +        for (i = 0; i < 4; i++) {
>> +            buffer[num_to_read - 1] = bytes[i];
>> +            num_to_read--;
>> +            if (num_to_read == 0) {
>> +                break;
>> +            }
>> +        }
>
> Copying backwards is a bit weird, and you could move the if out of the
> loop. Obviously not a big deal.
>
>    if (num_to_read <= 4) {
>        memcpy(buffer, bytes, num_to_read);
>        break;
>    } else {
>        memcpy(buffer, bytes, 4);
>        num_to_read -= 4;
>        buffer += 4;
>    }
>
> Actually, it would be better to rand_s directly into the output buffer
> until the remaining length is less than 4.

Done.

>> +    }
>> +
>> +    return MR_TRUE;
>> +}
>
> You didn't set *err_msg in this case.

Fixed.

> Do you know if it works with MinGW-w64?

Yes, I wrote it using MinGW-w64 ;-)  MinGW pretty much provides
direct access to the Windows API; it's not like Cygwin where there
is a heavy weight layer sitting on top of everything.

Revised diff below.

Julien.

-----------------------------

diff --git a/library/random.system_rng.m b/library/random.system_rng.m
index 59dd1d0c4..1feffaa78 100644
--- a/library/random.system_rng.m
+++ b/library/random.system_rng.m
@@ -119,6 +119,9 @@

  #include <errno.h>
  #include <stdint.h>
+
+// On Windows we need to ensure that _CRT_RAND_S is defined before stdlib.h
+// is included but this must be done in runtime/mercury_std.h.
  #include <stdlib.h>

  // The following macros define if the system random number exists on this
@@ -136,7 +139,7 @@
  //    PRNG, such as ChaCha20; it should _not_ be enabled on systems where
  //    arc4random() still uses RC4.
  //
-// ML_SYSRAND_IMPL_RAND_S (NYI)
+// ML_SYSRAND_IMPL_RAND_S
  //    the system RNG is implemented by calling the rand_s() function
  //    (Windows only).
  //
@@ -169,6 +172,8 @@
     #else
        #define ML_SYSRAND_IMPL_URANDOM
     #endif
+#elif defined(MR_WIN32)
+    #define ML_SYSRAND_IMPL_RAND_S
  #else
      #define ML_SYSRAND_IMPL_NONE
  #endif
@@ -675,6 +680,58 @@ ML_random_generate_bytes(ML_SystemRandomHandle handle,
      return MR_TRUE;
  }

+#elif defined(ML_SYSRAND_IMPL_RAND_S)
+
+ML_SystemRandomHandle
+ML_random_open(MR_Bool *succeeded, MR_String *err_msg)
+{
+    *succeeded = MR_TRUE;
+    *err_msg = MR_make_string_const(\"\");
+    return 0;
+}
+
+MR_Bool
+ML_random_close(ML_SystemRandomHandle handle, MR_String *err_msg)
+{
+    *err_msg = MR_make_string_const(\"\");
+    return MR_TRUE;
+}
+
+MR_Bool
+ML_random_generate_bytes(ML_SystemRandomHandle handle,
+    unsigned char *buffer, size_t len, MR_String *err_msg)
+{
+    int err;
+    unsigned int n;
+    size_t num_to_read = len;
+
+    while (num_to_read > 0) {
+        if (num_to_read < 4) {
+            err = rand_s(&n);
+            if (err != 0) {
+                goto rand_s_failure_handler;
+            }
+            MR_memcpy(buffer, (unsigned char *) &n, num_to_read);
+            break;
+        } else {
+            err = rand_s((unsigned int *) buffer);
+            if (err != 0) {
+                goto rand_s_failure_handler;
+            }
+            num_to_read -= 4;
+            buffer += 4;
+        }
+    }
+
+    *err_msg = MR_make_string_const(\"\");
+    return MR_TRUE;
+
+rand_s_failure_handler:
+
+    *err_msg = MR_make_string_const(\"rand_s failed\");
+    return MR_FALSE;
+}
+
  #else // ML_SYSRAND_IMPL_NONE

  ML_SystemRandomHandle
diff --git a/runtime/mercury_std.h b/runtime/mercury_std.h
index e34d0df36..5cdb77434 100644
--- a/runtime/mercury_std.h
+++ b/runtime/mercury_std.h
@@ -1,7 +1,7 @@
  // vim: ts=4 sw=4 expandtab ft=c

  // Copyright (C) 1993-1995, 1997-2005, 2011-2012 The University of Melbourne.
-// Copyright (C) 2014, 2016-2018 The Mercury team.
+// Copyright (C) 2014, 2016-2019, 2021 The Mercury team.
  // This file is distributed under the terms specified in COPYING.LIB.

  // std.h - "standard" [sic] definitions for C:
@@ -22,7 +22,14 @@
      #error "Mercury requires a system that provides stdint.h"
  #endif

+// On Windows we need to define _CRT_RAND_S *before* stdlib.h is included,
+// otherwise the declaration for rand_s() will not be visible.
+//
+#if defined(MR_WIN32) && !defined(_CRT_RAND_S)
+   #define _CRT_RAND_S
+#endif
  #include <stdlib.h> // for size_t
+
  #include <assert.h> // for assert()
  #include <errno.h>  // for EINTR
  #include <ctype.h>  // for isalnum(), etc.