Re: [PATCH] Bad timestamps on MinGW32

2022-11-03 Thread Eli Zaretskii
> From: Orgad Shaneh 
> Date: Thu, 3 Nov 2022 08:29:53 +0200
> Cc: bug-make@gnu.org
> 
> > > Do you mean that __MINGW_USE_VC2005_COMPAT has no effect on this
> > > platform?
> >
> > Indeed, it doesn't currently.  But I wouldn't count on that, some
> > future version could introduce it.
> 
> Is mingw.org still active?

It is, albeit much less so than MinGW64.

> Where can I get it?

   https://osdn.net/projects/mingw/releases

> > _stat64 _is_ supported, but it isn't the default.  So if a program
> > needs to use _stat64, it can only be safely linked against libraries
> > that were also compiled with stat redirected to _stat64, otherwise you
> > cannot safely pass 'struct stat' variables and pointers between the
> > program and those libraries.  And since _stat64 is not the default,
> > the chance that libraries against which you link used it is very
> > small, practically nil.
> 
> * Which libraries?

For example, Guile and all its dependencies.

There's also libmingwex, which provides some functions the MS C
runtime doesn't have.  libmingwex is basically linked into every MinGW
program, since the MinGW startup calls some of those extra runtime
functions.

> * Why should we care what other libraries do? Do we pass struct stat
> to other libraries, or only to system functions?

Are you sure 'struct stat' variables and pointers are never shared
with any libraries against which we link Make libraries?  How can you
be sure of that?  And even if today this is somehow true, will we
remember this tomorrow when we add new code to Make?

This is a time bomb waiting to go off.



Re: [PATCH] Bad timestamps on MinGW32

2022-11-03 Thread Orgad Shaneh
On Thu, Nov 3, 2022 at 8:07 AM Eli Zaretskii  wrote:
>
> > From: Orgad Shaneh 
> > Date: Wed, 2 Nov 2022 22:14:26 +0200
> > Cc: bug-make@gnu.org
> >
> > On Wed, Nov 2, 2022 at 7:04 PM Eli Zaretskii  wrote:
> > > > Fix by enabling _stat64 also for MinGW.
> > >
> > > Thanks, but this cannot be done for all MinGW builds.  There's
> > > mingw.org's MinGW (which is what I use), and its default is to use
> > > 32-bit time_t values.  If you use this change with that MinGW, Make
> > > will likely crash at run time, because various libraries it is linked
> > > against use a different time_t in stat calls.
> >
> > Hi, and thanks for the prompt reply.
> >
> > Do you mean that __MINGW_USE_VC2005_COMPAT has no effect on this
> > platform?
>
> Indeed, it doesn't currently.  But I wouldn't count on that, some
> future version could introduce it.

Is mingw.org still active? Where can I get it?

> > Or that it doesn't support _stat64?
>
> _stat64 _is_ supported, but it isn't the default.  So if a program
> needs to use _stat64, it can only be safely linked against libraries
> that were also compiled with stat redirected to _stat64, otherwise you
> cannot safely pass 'struct stat' variables and pointers between the
> program and those libraries.  And since _stat64 is not the default,
> the chance that libraries against which you link used it is very
> small, practically nil.

* Which libraries?
* Why should we care what other libraries do? Do we pass struct stat
to other libraries, or only to system functions?

- Orgad



Re: [PATCH] Bad timestamps on MinGW32

2022-11-03 Thread Eli Zaretskii
> From: Jeffrey Walton 
> Date: Wed, 2 Nov 2022 16:22:51 -0400
> Cc: bug-make@gnu.org
> 
> You might also be interested in __MINGW64__.

That's possible, but AFAIK it is only defined after including some
headers; the compiler itself doesn't define it.  Caveat emptor.

> I don't think it's a good idea to try to use MinGW or compiler
> versions as a proxy for 32- or 64-bit time_t. Things are in flux to
> fix the y2038 problem. The debian-arm and debian-powerpc lists are
> having discussions about it now. You will surely encounter 64-bit
> time_t on 32-bit machines eventually.

32-bit Windows have 64-bit time_t variant, but to use it you need to
redirect some function to non-default variants.  'stat' is on of those
functions.

> A first class configure test that checks for the size of time_t will
> probably be your best choice.

As I explained in my other mail, the issue here is not to know whether
the platform _can_ support 64-bit time data types, the issue is what
is the default, because libraries against Make is linked were most
probably compiled with that default.



Re: [PATCH] Bad timestamps on MinGW32

2022-11-03 Thread Eli Zaretskii
> From: Orgad Shaneh 
> Date: Wed, 2 Nov 2022 22:14:26 +0200
> Cc: bug-make@gnu.org
> 
> On Wed, Nov 2, 2022 at 7:04 PM Eli Zaretskii  wrote:
> > > Fix by enabling _stat64 also for MinGW.
> >
> > Thanks, but this cannot be done for all MinGW builds.  There's
> > mingw.org's MinGW (which is what I use), and its default is to use
> > 32-bit time_t values.  If you use this change with that MinGW, Make
> > will likely crash at run time, because various libraries it is linked
> > against use a different time_t in stat calls.
> 
> Hi, and thanks for the prompt reply.
> 
> Do you mean that __MINGW_USE_VC2005_COMPAT has no effect on this
> platform?

Indeed, it doesn't currently.  But I wouldn't count on that, some
future version could introduce it.

> Or that it doesn't support _stat64?

_stat64 _is_ supported, but it isn't the default.  So if a program
needs to use _stat64, it can only be safely linked against libraries
that were also compiled with stat redirected to _stat64, otherwise you
cannot safely pass 'struct stat' variables and pointers between the
program and those libraries.  And since _stat64 is not the default,
the chance that libraries against which you link used it is very
small, practically nil.

The same goes for MinGW64, btw: you can only safely redirect stat to
_stat64 there if MinGW64 does that by default.  (I think it does, but
I'm not sure.)

> > So this condition:
> >
> > > -#if defined(_MSC_VER) && _MSC_VER > 1200
> > > +#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER > 1200)
> >
> > must be rewritten to catch only MinGW64 builds.  Which would mean a
> > more fine-grained testing of the value of __MINGW_MAJOR_VERSION etc.
> 
> I'll try to find something. But first I'll need an answer for the
> previous question, so I know which changes to look for in mingw.

I hope I answered them.



Re: [PATCH] Bad timestamps on MinGW32

2022-11-02 Thread Paul Smith
On Wed, 2022-11-02 at 16:22 -0400, Jeffrey Walton wrote:
> A first class configure test that checks for the size of time_t will
> probably be your best choice.

Unfortunately we don't (always) run configure on Windows (it's possible
to build GNU make on Windows systems where you can't run configure
because you don't have a POSIX environment).

So, a configure check isn't the best choice.  We'd prefer a
preprocessor test that can detect this situation.



Re: [PATCH] Bad timestamps on MinGW32

2022-11-02 Thread Jeffrey Walton
On Wed, Nov 2, 2022 at 4:15 PM Orgad Shaneh  wrote:
>
> On Wed, Nov 2, 2022 at 7:04 PM Eli Zaretskii  wrote:
> > > Fix by enabling _stat64 also for MinGW.
> >
> > Thanks, but this cannot be done for all MinGW builds.  There's
> > mingw.org's MinGW (which is what I use), and its default is to use
> > 32-bit time_t values.  If you use this change with that MinGW, Make
> > will likely crash at run time, because various libraries it is linked
> > against use a different time_t in stat calls.
>
> Hi, and thanks for the prompt reply.
>
> Do you mean that __MINGW_USE_VC2005_COMPAT has no effect on this
> platform? Or that it doesn't support _stat64? Or something else?
>
> > So this condition:
> >
> > > -#if defined(_MSC_VER) && _MSC_VER > 1200
> > > +#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER > 1200)
> >
> > must be rewritten to catch only MinGW64 builds.  Which would mean a
> > more fine-grained testing of the value of __MINGW_MAJOR_VERSION etc.
>
> I'll try to find something. But first I'll need an answer for the
> previous question, so I know which changes to look for in mingw.

You might also be interested in __MINGW64__.

I don't think it's a good idea to try to use MinGW or compiler
versions as a proxy for 32- or 64-bit time_t. Things are in flux to
fix the y2038 problem. The debian-arm and debian-powerpc lists are
having discussions about it now. You will surely encounter 64-bit
time_t on 32-bit machines eventually.

A first class configure test that checks for the size of time_t will
probably be your best choice.

Jeff



Re: [PATCH] Bad timestamps on MinGW32

2022-11-02 Thread Orgad Shaneh
On Wed, Nov 2, 2022 at 7:04 PM Eli Zaretskii  wrote:
> > Fix by enabling _stat64 also for MinGW.
>
> Thanks, but this cannot be done for all MinGW builds.  There's
> mingw.org's MinGW (which is what I use), and its default is to use
> 32-bit time_t values.  If you use this change with that MinGW, Make
> will likely crash at run time, because various libraries it is linked
> against use a different time_t in stat calls.

Hi, and thanks for the prompt reply.

Do you mean that __MINGW_USE_VC2005_COMPAT has no effect on this
platform? Or that it doesn't support _stat64? Or something else?

> So this condition:
>
> > -#if defined(_MSC_VER) && _MSC_VER > 1200
> > +#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER > 1200)
>
> must be rewritten to catch only MinGW64 builds.  Which would mean a
> more fine-grained testing of the value of __MINGW_MAJOR_VERSION etc.

I'll try to find something. But first I'll need an answer for the
previous question, so I know which changes to look for in mingw.

- Orgad



Re: [PATCH] Bad timestamps on MinGW32

2022-11-02 Thread Eli Zaretskii
> From: Orgad Shaneh 
> Date: Wed, 2 Nov 2022 18:32:49 +0200
> 
> Commit 01142a53c9d (Add support for intmax_t) added support for 64-bit
> time_t by defining __MINGW_USE_VC2005_COMPAT. But this only works with
> _stat64 as expected. When stat is used on 32-bit systems, it returns a
> bad timestamp (for example, 72586185920376753).
> 
> This triggers the following errors every time make is executed:
> mingw32-make: Warning: File 'Makefile' has modification time 7.3e+16 s
> in the future
> mingw32-make: warning:  Clock skew detected.  Your build may be incomplete.
> 
> and of course, dependencies are completely broken.
> 
> Fix by enabling _stat64 also for MinGW.

Thanks, but this cannot be done for all MinGW builds.  There's
mingw.org's MinGW (which is what I use), and its default is to use
32-bit time_t values.  If you use this change with that MinGW, Make
will likely crash at run time, because various libraries it is linked
against use a different time_t in stat calls.

So this condition:

> -#if defined(_MSC_VER) && _MSC_VER > 1200
> +#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER > 1200)

must be rewritten to catch only MinGW64 builds.  Which would mean a
more fine-grained testing of the value of __MINGW_MAJOR_VERSION etc.