[Patch] Implement sys::Process::GetRandomNumber on Windows

[Stephan Tolksdorf]

Aaron Ballman wrote:
> Aside from minor nits, LGTM. Nits below:

Thank you for the review! I've attached an updated patch that 
incorporates the suggested changes.

- Stephan

>
>>  From b2976da4fc38c9fb41498915af3d6eee8bac7267 Mon Sep 17 00:00:00 2001
>> From: Stephan Tolksdorf <st at quanttec.com>
>> Date: Mon, 3 Feb 2014 21:25:01 +0100
>> Subject: Implement llvm::sys::Process::GetRandomNumber() for Windows
>>
>> This function was already implemented for Unix. The implementation uses the
>> rand_s function, which according to MSDN is available on Windows XP and later.
>> libc++ also uses rand_s for implementing random_device. I've checked
>> that mingw-w64 supports rand_s.
>> ---
>>   lib/Support/Process.cpp           | 4 ++++
>>   lib/Support/Windows/Process.inc   | 7 +++++++
>>   unittests/Support/ProcessTest.cpp | 6 ++++++
>>   3 files changed, 17 insertions(+)
>>
>> diff --git a/lib/Support/Process.cpp b/lib/Support/Process.cpp
>> index d5168f0..009c7c8 100644
>> --- a/lib/Support/Process.cpp
>> +++ b/lib/Support/Process.cpp
>> @@ -9,12 +9,16 @@
>>   //
>>   //  This header file implements the operating system Process concept.
>>   //
>>   //===----------------------------------------------------------------------===//
>>
>>   #include "llvm/Config/config.h"
>> +#if LLVM_ON_WIN32
>> +# define _CRT_RAND_S // makes stdlib.h declare the rand_s function
>
> LLVM comments are usually complete sentences (capitalization, period, etc).
>
>> +# include <stdlib.h>
>> +#endif
>>   #include "llvm/Support/ErrorHandling.h"
>>   #include "llvm/Support/Process.h"
>>
>>   using namespace llvm;
>>   using namespace sys;
>>
>> diff --git a/lib/Support/Windows/Process.inc b/lib/Support/Windows/Process.inc
>> index 750097e..73ce97a 100644
>> --- a/lib/Support/Windows/Process.inc
>> +++ b/lib/Support/Windows/Process.inc
>> @@ -357,6 +357,13 @@ const char *Process::OutputReverse() {
>>
>>   const char *Process::ResetColor() {
>>     if (UseANSI) return "\033[0m";
>>     SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), defaultColors());
>>     return 0;
>>   }
>> +
>> +unsigned Process::GetRandomNumber() {
>> +  unsigned result;
>> +  const errno_t ec = rand_s(&result); // supported in Win XP and later
>
> I would remove the comment; also, result should be declared to match
> rand_s's signature (so unsigned int instead of vanilla unsigned).
>
>> +  assert(ec == 0 && "rand_s failed");
>> +  return result;
>> +}
>> diff --git a/unittests/Support/ProcessTest.cpp b/unittests/Support/ProcessTest.cpp
>> index af6a6f9..387d5d3 100644
>> --- a/unittests/Support/ProcessTest.cpp
>> +++ b/unittests/Support/ProcessTest.cpp
>> @@ -36,12 +36,18 @@ TEST(ProcessTest, SelfProcess) {
>>     EXPECT_LT(TimeValue::MinTime, process::get_self()->get_system_time());
>>     EXPECT_GT(TimeValue::MaxTime, process::get_self()->get_system_time());
>>     EXPECT_LT(TimeValue::MinTime, process::get_self()->get_wall_time());
>>     EXPECT_GT(TimeValue::MaxTime, process::get_self()->get_wall_time());
>>   }
>>
>> +TEST(ProcessTest, GetRandomNumberTest) {
>> +  const unsigned r1 = Process::GetRandomNumber();
>> +  const unsigned r2 = Process::GetRandomNumber();
>> +  EXPECT_NE((r1 | r2), 0); // 0 should be extremely unlikely
>> +}
>> +
>>   #ifdef _MSC_VER
>>   #define setenv(name, var, ignore) _putenv_s(name, var)
>>   #endif
>>
>>   #if HAVE_SETENV || _MSC_VER
>>   TEST(ProcessTest, Basic) {
>> --
>> 1.7.11.msysgit.1
>
> ~Aaron
>
> On Mon, Feb 3, 2014 at 7:03 PM, Stephan Tolksdorf <st at quanttec.com> wrote:
>> Hi,
>>
>> This is a little patch that implements sys::Process::GetRandomNumber on
>> Windows. Currently this function only has a Unix implementation.
>>
>> The implementation uses the s_rand function, which is available on Windows
>> XP and later. It's the same function that libc++ uses for implementing
>> random_device on Windows.
>>
>> - Stephan
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>

-------------- next part --------------
Implement sys::Process::GetRandomNumber() on Windows

This function was already implemented for Unix. The implementation uses the
rand_s function, which according to MSDN is available on Windows XP and later.
libc++ also uses rand_s for implementing random_device. I've checked
that mingw-w64 supports rand_s.
---
 lib/Support/Process.cpp           | 5 +++++
 lib/Support/Windows/Process.inc   | 7 +++++++
 unittests/Support/ProcessTest.cpp | 7 +++++++
 3 files changed, 19 insertions(+)

diff --git a/lib/Support/Process.cpp b/lib/Support/Process.cpp
index d5168f0..41221de 100644
--- a/lib/Support/Process.cpp
+++ b/lib/Support/Process.cpp
@@ -9,12 +9,17 @@
 //
 //  This header file implements the operating system Process concept.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Config/config.h"
+#if LLVM_ON_WIN32
+  // This define makes stdlib.h declare the rand_s function.
+# define _CRT_RAND_S
+# include <stdlib.h>
+#endif
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Process.h"
 
 using namespace llvm;
 using namespace sys;
 
diff --git a/lib/Support/Windows/Process.inc b/lib/Support/Windows/Process.inc
index 750097e..16e4092 100644
--- a/lib/Support/Windows/Process.inc
+++ b/lib/Support/Windows/Process.inc
@@ -357,6 +357,13 @@ const char *Process::OutputReverse() {
 
 const char *Process::ResetColor() {
   if (UseANSI) return "\033[0m";
   SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), defaultColors());
   return 0;
 }
+
+unsigned Process::GetRandomNumber() {
+  unsigned int result;
+  const errno_t ec = rand_s(&result);
+  assert(ec == 0 && "rand_s failed");
+  return result;
+}
diff --git a/unittests/Support/ProcessTest.cpp b/unittests/Support/ProcessTest.cpp
index af6a6f9..19694e5 100644
--- a/unittests/Support/ProcessTest.cpp
+++ b/unittests/Support/ProcessTest.cpp
@@ -36,12 +36,19 @@ TEST(ProcessTest, SelfProcess) {
   EXPECT_LT(TimeValue::MinTime, process::get_self()->get_system_time());
   EXPECT_GT(TimeValue::MaxTime, process::get_self()->get_system_time());
   EXPECT_LT(TimeValue::MinTime, process::get_self()->get_wall_time());
   EXPECT_GT(TimeValue::MaxTime, process::get_self()->get_wall_time());
 }
 
+TEST(ProcessTest, GetRandomNumberTest) {
+  const unsigned r1 = Process::GetRandomNumber();
+  const unsigned r2 = Process::GetRandomNumber();
+  // It should be extremely unlikely that both r1 and r2 are 0.
+  EXPECT_NE((r1 | r2), 0);
+}
+
 #ifdef _MSC_VER
 #define setenv(name, var, ignore) _putenv_s(name, var)
 #endif
 
 #if HAVE_SETENV || _MSC_VER
 TEST(ProcessTest, Basic) {
-- 
1.7.11.msysgit.1