[llvm] r200767 - Implemented support for Process::GetRandomNumber on Windows.

Tue Feb 11 09:12:07 PST 2014

Sorry for not seeing this thread earlier. I've got to fix my email 
configuration...

Aaron, thanks a lot for jumping and helping out!

As I wrote in my original patch email: I had tested the patch with 
(multiple versions) mingw-w64, and libc++ uses the function in the same way.

One tricky aspect of rand_s is that you have to make sure that 
_CRT_RAND_S is defined before stdlib.h is included the first time.

What versions of MinGW are LLVM, Clang and libc++ actually trying to 
support?

- Stephan

On 11.02.14 17:26, Aaron Ballman wrote:
> That doesn't appear to be the case for the MinGW bot Galina had -- it
> was also lacking errno_t, so my guess is that it's an age problem. The
> new implementation skirts the issue by using the underlying Win32 APIs
> to generate the random numbers.
>
> ~Aaron
>
> On Tue, Feb 11, 2014 at 1:48 AM, Yaron Keren <yaron.keren at gmail.com> wrote:
>> Hi,
>>
>> Sorry for not catching this thread earlier, MinGW does have rand_s if
>> _CRT_RAND_S is defined.
>>  From stdio.h:
>>
>> #ifdef _CRT_RAND_S
>>    _CRTIMP errno_t __cdecl rand_s(unsigned int *randomValue);
>> #endif
>>
>> libcxx already uses rand_s in random_device and compiles with MinGW, here is
>> the discussion:
>>
>>
>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131007/090656.html
>>
>> Yaron
>>
>>
>>
>>
>>
>> 2014-02-11 5:46 GMT+02:00 Aaron Ballman <aaron at aaronballman.com>:
>>>
>>> This is hopefully resolved in r201124; I reimplemented the original
>>> patch using CryptGenRandom, which is effectively what rand_s() uses
>>> internally anyway.
>>>
>>> ~Aaron
>>>
>>> On Mon, Feb 10, 2014 at 8:52 PM, Aaron Ballman <aaron at aaronballman.com>
>>> wrote:
>>>> It appears there is no rand_s() implementation for MinGW 32, so the
>>>> question becomes whether we back this change out, or fall back on
>>>> rand(). Unfortunately, r185126 is relying on this, and I'm loathe to
>>>> start backing out other people's work because of this. So instead, I
>>>> will gin up a replacement for MinGW 32 and hopefully Stephan can
>>>> either come up with a better fix, or mine will be sufficient for the
>>>> task.
>>>>
>>>> ~Aaron
>>>>
>>>> On Mon, Feb 10, 2014 at 2:15 PM, Aaron Ballman <aaron at aaronballman.com>
>>>> wrote:
>>>>> Thank you for pointing this out, Galina and sorry for the difficulties!
>>>>>
>>>>> Stephan, would you be able to look into this?
>>>>>
>>>>> ~Aaron
>>>>>
>>>>> On Mon, Feb 10, 2014 at 2:05 PM, Galina Kistanova
>>>>> <gkistanova at gmail.com> wrote:
>>>>>> Hi Aaron,
>>>>>>
>>>>>> Seems this revision broke clang-native-mingw32-win7 builder:
>>>>>>
>>>>>> http://lab.llvm.org:8011/builders/clang-native-mingw32-win7/builds/5557
>>>>>>
>>>>>> The last successful build was for revision 200765:
>>>>>>
>>>>>> http://lab.llvm.org:8011/builders/clang-native-mingw32-win7/builds/5578
>>>>>> Please have a look at it.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Galina
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Feb 4, 2014 at 6:49 AM, Aaron Ballman <aaron at aaronballman.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Author: aaronballman
>>>>>>> Date: Tue Feb  4 08:49:21 2014
>>>>>>> New Revision: 200767
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=200767&view=rev
>>>>>>> Log:
>>>>>>> Implemented support for Process::GetRandomNumber on Windows.
>>>>>>>
>>>>>>> Patch thanks to Stephan Tolksdorf!
>>>>>>>
>>>>>>> Modified:
>>>>>>>      llvm/trunk/lib/Support/Process.cpp
>>>>>>>      llvm/trunk/lib/Support/Windows/Process.inc
>>>>>>>      llvm/trunk/unittests/Support/ProcessTest.cpp
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/Support/Process.cpp
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Process.cpp?rev=200767&r1=200766&r2=200767&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/Support/Process.cpp (original)
>>>>>>> +++ llvm/trunk/lib/Support/Process.cpp Tue Feb  4 08:49:21 2014
>>>>>>> @@ -12,6 +12,11 @@
>>>>>>>
>>>>>>>
>>>>>>> //===----------------------------------------------------------------------===//
>>>>>>>
>>>>>>>   #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"
>>>>>>>
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/Support/Windows/Process.inc
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Process.inc?rev=200767&r1=200766&r2=200767&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/Support/Windows/Process.inc (original)
>>>>>>> +++ llvm/trunk/lib/Support/Windows/Process.inc Tue Feb  4 08:49:21
>>>>>>> 2014
>>>>>>> @@ -360,3 +360,10 @@ const char *Process::ResetColor() {
>>>>>>>     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;
>>>>>>> +}
>>>>>>>
>>>>>>> Modified: llvm/trunk/unittests/Support/ProcessTest.cpp
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ProcessTest.cpp?rev=200767&r1=200766&r2=200767&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/unittests/Support/ProcessTest.cpp (original)
>>>>>>> +++ llvm/trunk/unittests/Support/ProcessTest.cpp Tue Feb  4 08:49:21
>>>>>>> 2014
>>>>>>> @@ -39,6 +39,13 @@ TEST(ProcessTest, SelfProcess) {
>>>>>>>     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
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> llvm-commits mailing list
>>>>>>> llvm-commits at cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Thanks
>>>>>>
>>>>>> Galina
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>