| Message ID | CAF1jjLvoGe-e0VsRTikY-s6YB5Kts3p9pi3qWrX+Ls+Rgvt_XQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sun, Mar 12, 2017 at 1:26 PM, NightStrike <nightstrike@gmail.com> wrote: > On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist > <blomqvist.janne@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 <jb@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. > > 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. Testsuite run completed with no change in results: === gfortran Summary === # of expected passes 44633 # of unexpected failures 55 # of unexpected successes 6 # of expected failures 85 # of unsupported tests 187 /tmp/build/gcc/testsuite/gfortran/../../gfortran version 7.0.1 20170312 (experimental) (GCC) make[1]: Leaving directory '/tmp/build/gcc' Those 55 unexpected failures are unrelated to this change, and occur before and after.
On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstrike@gmail.com> wrote: > On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist > <blomqvist.janne@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 <jb@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 > 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.
On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist <blomqvist.janne@gmail.com> wrote: > On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstrike@gmail.com> wrote: >> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist >> <blomqvist.janne@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 <jb@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 > >> 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
On Mon, Mar 13, 2017 at 5:01 AM, Janne Blomqvist <blomqvist.janne@gmail.com> wrote: > On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist > <blomqvist.janne@gmail.com> wrote: >> On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstrike@gmail.com> wrote: >>> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist >>> <blomqvist.janne@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 <jb@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.
On Tue, Mar 14, 2017 at 6:32 AM, NightStrike <nightstrike@gmail.com> wrote: > On Mon, Mar 13, 2017 at 5:01 AM, Janne Blomqvist > <blomqvist.janne@gmail.com> wrote: >> On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist >> <blomqvist.janne@gmail.com> wrote: >>> On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstrike@gmail.com> wrote: >>>> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist >>>> <blomqvist.janne@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 <jb@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. Ok for trunk. You said on IRC that cygwin is a rolling release system and they don't support old releases, and the bug that prompted this was fixed 4 years ago, so I agree we don't need to support that anymore.
On Tue, Mar 14, 2017 at 10:05 AM, Janne Blomqvist <blomqvist.janne@gmail.com> wrote: > On Tue, Mar 14, 2017 at 6:32 AM, NightStrike <nightstrike@gmail.com> wrote: >> On Mon, Mar 13, 2017 at 5:01 AM, Janne Blomqvist >> <blomqvist.janne@gmail.com> wrote: >>> On Sun, Mar 12, 2017 at 10:46 PM, Janne Blomqvist >>> <blomqvist.janne@gmail.com> wrote: >>>> On Sun, Mar 12, 2017 at 7:26 PM, NightStrike <nightstrike@gmail.com> wrote: >>>>> On Mon, Feb 27, 2017 at 6:15 AM, Janne Blomqvist >>>>> <blomqvist.janne@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 <jb@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. > > Ok for trunk. > > You said on IRC that cygwin is a rolling release system and they don't > support old releases, and the bug that prompted this was fixed 4 years > ago, so I agree we don't need to support that anymore. As 'NightStrike' doesn't have commit access, I have committed r246162. https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=246162 Don't use Win32 functions on CYGWIN. This was a workaround for a cygwin bug which was fixed 4 years ago, and cygwin hasn't supported affected versions for a long time. 2017-03-15 NightStrike <nightstrike@gmail.com> Janne Blomqvist <jb@gcc.gnu.org> * intrinsics/random.c (getosrandom): Remove check for __CYGWIN__ preprocessor flag. * intrinsics/system_clock.c: Likewise. (system_clock_4): Likewise. (system_clock_8): Likewise. * intrinsics/time_1.h: Don't include windows.h if __CYGWIN__ is defined.
Index: ../gccsvn/libgfortran/intrinsics/random.c =================================================================== --- ../gccsvn/libgfortran/intrinsics/random.c (revision 246075) +++ ../gccsvn/libgfortran/intrinsics/random.c (working copy) @@ -304,7 +304,7 @@ getosrandom (void *buf, size_t buflen) { /* rand_s is available in MinGW-w64 but not plain MinGW. */ -#if defined(__MINGW64_VERSION_MAJOR) && !defined(__CYGWIN__) +#ifdef __MINGW64_VERSION_MAJOR unsigned int* b = buf; for (unsigned i = 0; i < buflen / sizeof (unsigned int); i++) rand_s (&b[i]); Index: ../gccsvn/libgfortran/intrinsics/system_clock.c =================================================================== --- ../gccsvn/libgfortran/intrinsics/system_clock.c (revision 246075) +++ ../gccsvn/libgfortran/intrinsics/system_clock.c (working copy) @@ -29,7 +29,7 @@ #include "time_1.h" -#if !defined(__MINGW32__) && !defined(__CYGWIN__) +#if !defined(__MINGW32__) /* POSIX states that CLOCK_REALTIME must be present if clock_gettime is available, others are optional. */ @@ -121,7 +121,7 @@ system_clock_4 (GFC_INTEGER_4 *count, GFC_INTEGER_4 *count_rate, GFC_INTEGER_4 *count_max) { -#if defined(__MINGW32__) || defined(__CYGWIN__) +#if defined(__MINGW32__) if (count) { /* Use GetTickCount here as the resolution and range is @@ -174,7 +174,7 @@ system_clock_8 (GFC_INTEGER_8 *count, GFC_INTEGER_8 *count_rate, GFC_INTEGER_8 *count_max) { -#if defined(__MINGW32__) || defined(__CYGWIN__) +#if defined(__MINGW32__) LARGE_INTEGER cnt; LARGE_INTEGER freq; bool fail = false; Index: ../gccsvn/libgfortran/intrinsics/time_1.h =================================================================== --- ../gccsvn/libgfortran/intrinsics/time_1.h (revision 246075) +++ ../gccsvn/libgfortran/intrinsics/time_1.h (working copy) @@ -101,7 +101,7 @@ CPU_TIME intrinsics. Returns 0 for success or -1 if no CPU time could be computed. */ -#if defined(__MINGW32__) || defined(__CYGWIN__) +#if defined(__MINGW32__) #define WIN32_LEAN_AND_MEAN #include <windows.h>