On Mon, Mar 13, 2017 at 5:01 AM, Janne Blomqvist
<blomqvist.ja...@gmail.com> wrote:
> On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist
> <blomqvist.ja...@gmail.com> wrote:
>> On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstr...@gmail.com> wrote:
>>> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist
>>> <blomqvist.ja...@gmail.com> wrote:
>>>> Don't try to use rand_s on CYGWIN
>>>>
>>>> CYGWIN seems to include _mingw.h and thus __MINGW64_VERSION_MAJOR is
>>>> defined even though rand_s is not available. Thus add an extra check
>>>> for __CYGWIN__.
>>>>
>>>> Thanks to Tim Prince and Nightstrike for bringing this issue to my 
>>>> attention.
>>>>
>>>> Committed as r245755.
>>>>
>>>> 2017-02-27  Janne Blomqvist  <j...@gcc.gnu.org>
>>>>
>>>>     * intrinsics/random.c (getosrandom): Don't try to use rand_s on
>>>>     CYGWIN.
>>>
>>> 1) There was no patch attached to the email.
>>
>> Oh, sorry. Well, you can see it at
>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=245755

Thanks.  You know, this is actually better than attaching a patch (as
a general thing for emails sent after things are already committed),
as there can be no difference between what was emailed and what was
committed.

>>> 2) As mentioned on IRC, I don't think this is the right fix.  The real
>>> problem is that time_1.h includes windows.h on CYGWIN, which it
>>> shouldn't be doing.  This then pollutes the translation unit with all
>>> sorts of MINGW-isms that aren't exactly appropriate for cygwin.
>>> Removing the include in time_1.h and then adjusting a few ifdefs in
>>> system_clock.c that also had the same bug fixes the problem.  The
>>> testsuite is running right now on this.
>>
>> It used to be something like that, but IIRC there were some complaints
>> about SYSTEM_CLOCK on cygwin that were due to the cygwin
>> clock_gettime() or something like that, and after some discussion with
>> someone who knows something about cygwin/mingw (since you apparently
>> have no memory of it, I guess it was Kai), it was decided to use
>> GetTickCount & QPC also on cygwin.
>
> I searched a bit, and it seems the culprit is the thread starting at
>
> https://gcc.gnu.org/ml/fortran/2013-04/msg00033.html
>
> So it turned out that clock_gettime(CLOCK_MONOTONIC, ...) always
> returned 0 on cygwin, hence the code was changed to use the windows
> API functions GetTickCount and QPC also on cygwin at
>
> https://gcc.gnu.org/ml/fortran/2013-04/msg00124.html
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56919

That all led me to this thread:

https://cygwin.com/ml/cygwin/2013-04/msg00184.html

which led me to:

https://cygwin.com/ml/cygwin/2013-04/msg00191.html

where Corinna fixed Angelo's original issue that sparked the whole
thing.  So, from my perspective, Cygwin hasn't had this problem in
about 4 years.

To be complete, though, I took Angelo's original code and compiled it
with a GCC built with the change I suggested, and I received this:

$ ./a.exe
   9.50646996E-02  0.435180306      0.939791977      0.851783216
0.308901191      0.447312355      0.766363919      0.163527727
1.25432014E-02

$ ./a.exe
  0.445786893       9.30446386E-03  0.880381405      0.123394549
1.23693347E-02  0.485788047      0.623361290      0.921140671
0.119319797

$ ./a.exe
  0.378171027      0.439836979      0.440582573       1.17767453E-02
0.427448869      0.530438244      0.182108700      0.147965968
0.668991745

$ ./a.exe
   2.73125172E-02  0.916011810      0.854288757      0.913502872
0.508077919      0.210798383       8.76839161E-02  0.120695710
0.337186754


I then tried Janus' enhanced version from
https://gcc.gnu.org/ml/fortran/2013-04/msg00034.html

$ ./a.exe
 n=          33
 clock:   744091787
 seed:    744091787   744091824   744091861   744091898   744091935
744091972   744092009   744092046   744092083   744092120   744092157
 744092194   744092231   744092268   744092305   744092342   744092379
  744092416   744092453   744092490   744092527   744092564
744092601   744092638   744092675   744092712   744092749   744092786
 744092823   744092860   744092897   744092934   744092971
 random:   0.801897824      0.180594921      0.280960143
8.65536928E-03  0.122029424      0.473693788      0.161536098
0.228073180      0.441518903

$ ./a.exe
 n=          33
 clock:   744093409
 seed:    744093409   744093446   744093483   744093520   744093557
744093594   744093631   744093668   744093705   744093742   744093779
 744093816   744093853   744093890   744093927   744093964   744094001
  744094038   744094075   744094112   744094149   744094186
744094223   744094260   744094297   744094334   744094371   744094408
 744094445   744094482   744094519   744094556   744094593
 random:   0.164107382      0.762304962      0.511664748
0.700617492      0.692375600      0.207925439      0.920203805
0.160881400      0.339902878

$ ./a.exe
 n=          33
 clock:   744094930
 seed:    744094930   744094967   744095004   744095041   744095078
744095115   744095152   744095189   744095226   744095263   744095300
 744095337   744095374   744095411   744095448   744095485   744095522
  744095559   744095596   744095633   744095670   744095707
744095744   744095781   744095818   744095855   744095892   744095929
 744095966   744096003   744096040   744096077   744096114
 random:   0.433895171      0.989695787      0.305223107
0.387590647      0.673205614      0.539944470      0.849159062
0.811356246      0.888609290

$ ./a.exe
 n=          33
 clock:   744096561
 seed:    744096561   744096598   744096635   744096672   744096709
744096746   744096783   744096820   744096857   744096894   744096931
 744096968   744097005   744097042   744097079   744097116   744097153
  744097190   744097227   744097264   744097301   744097338
744097375   744097412   744097449   744097486   744097523   744097560
 744097597   744097634   744097671   744097708   744097745
 random:   0.224010825      0.763803065      0.111726880
0.500481725       6.07219338E-02  0.413555145      0.896298766
0.142876744      0.286089420


And finally, the output of
https://gcc.gnu.org/ml/fortran/2013-04/msg00038.html  which you
requested in https://gcc.gnu.org/ml/fortran/2013-04/msg00207.html as a
test for the original fix:

$ ./a.exe
   744248371        1000  2147483647
      744248371346087           1000000000  9223372036854775807

$ ./a.exe
   744249100        1000  2147483647
      744249100677540           1000000000  9223372036854775807

$ ./a.exe
   744249678        1000  2147483647
      744249678546099           1000000000  9223372036854775807

$ ./a.exe
   744250116        1000  2147483647
      744250116491405           1000000000  9223372036854775807



So............  I know this was a long email, but it seems to me that
the better upstream fix went in a few weeks before these other
libgfortran changes, and they allow for a libgfortran that's untainted
by windows.h.  I really think this is the better, safer approach that
will prevent future errors, and remove the need for CYGWIN macro
checks in multiple places.

Reply via email to