Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-12-10 Thread Alan Bateman 

On 10/12/2018 11:05, Magnus Ihse Bursie wrote:



On 2018-12-10 11:56, Nick Gasson (Arm Technology China) wrote:

Hi,

Any update on this one? Or do we want to give up on it until JDK-8165620
is implemented?
I think Alan reviewed it as OK with just a minor fix, and offered to 
sponsor it for you.
That's right, the change broke the macOS build. It's on my list to 
re-test and sponsor this week.


-Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-12-10 Thread Magnus Ihse Bursie 




On 2018-12-10 11:56, Nick Gasson (Arm Technology China) wrote:

Hi,

Any update on this one? Or do we want to give up on it until JDK-8165620
is implemented?
I think Alan reviewed it as OK with just a minor fix, and offered to 
sponsor it for you.


Then the discussion started about some major changes, rewriting 
configure to provide individual support for every possible system 
function... I recommend that you go with the fix that solves your problem.


In fact, I disagree with the premise of JDK-8165620. I think it's better 
to do what you're doing here, to update the source code, so that it is 
clear that we know that we're handing 64-bit data.


/Magnus


Thanks,
Nick


On 28/11/2018 11:33, Martin Buchholz wrote:


On Tue, Nov 27, 2018 at 7:25 PM, Nick Gasson mailto:nick.gas...@arm.com>> wrote:

 > I missed one thing then looking at this. In TimeZone_md.c it should be
 > #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
 > I need to change this one line before pushing.

 Hi Alan,

 Yes feel free to modify it. I think looking at the at other files
 with these #defines more closely, is it the case that the #ifndef
 MACOSX check is only required for statvfs64? As in
 e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
 this:

 #if defined(_ALLBSD_SOURCE)
#define stat64 stat
#define lstat64 lstat
#define fstat64 fstat
 #endif

 I don't have access to any OS X machines to test unfortunately.

 But I wonder if a better way to handle this is to check for the
 presence of the stat64 functions in the configure script, and
 then we could just write something like this, which would be a
 lot clearer:

 #if !defined(HAVE_STAT64)
#define stat64 stat
 #endif



The best way is to implement

https://bugs.openjdk.java.net/browse/JDK-8165620

"Entire JDK should be built with -D_FILE_OFFSET_BITS=64"

but yes, another good way is to do as you suggest, have configure define
HAVE_ for all known functions with a 64-bit variant and then put
them into a header file with proper ifdefs for platforms that don't have
them.

You could also "simply" add
#define _FILE_OFFSET_BITS 64
but you have to do it for cliques of files that share ambiguously sized
data simultaneously.

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-12-10 Thread Nick Gasson (Arm Technology China) 
Hi,

Any update on this one? Or do we want to give up on it until JDK-8165620 
is implemented?

Thanks,
Nick


On 28/11/2018 11:33, Martin Buchholz wrote:
> 
> 
> On Tue, Nov 27, 2018 at 7:25 PM, Nick Gasson  > wrote:
> 
> > I missed one thing then looking at this. In TimeZone_md.c it should be
> > #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> > I need to change this one line before pushing.
> 
> Hi Alan,
> 
> Yes feel free to modify it. I think looking at the at other files
> with these #defines more closely, is it the case that the #ifndef
> MACOSX check is only required for statvfs64? As in
> e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
> this:
> 
> #if defined(_ALLBSD_SOURCE)
>    #define stat64 stat
>    #define lstat64 lstat
>    #define fstat64 fstat
> #endif
> 
> I don't have access to any OS X machines to test unfortunately.
> 
> But I wonder if a better way to handle this is to check for the
> presence of the stat64 functions in the configure script, and
> then we could just write something like this, which would be a
> lot clearer:
> 
> #if !defined(HAVE_STAT64)
>    #define stat64 stat
> #endif
> 
> 
> 
> The best way is to implement
> 
> https://bugs.openjdk.java.net/browse/JDK-8165620 
> 
> "Entire JDK should be built with -D_FILE_OFFSET_BITS=64"
> 
> but yes, another good way is to do as you suggest, have configure define 
> HAVE_ for all known functions with a 64-bit variant and then put 
> them into a header file with proper ifdefs for platforms that don't have 
> them.
> 
> You could also "simply" add
> #define _FILE_OFFSET_BITS 64
> but you have to do it for cliques of files that share ambiguously sized 
> data simultaneously.

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-29 Thread Nick Gasson 
Hi Alan, Martin,

Do you want me to make any more changes to this patch, or is it OK to go in 
with just the modified #ifdef?

Thanks,
Nick

From: Martin Buchholz 
Sent: 28 November 2018 11:33
To: Nick Gasson 
Cc: Alan Bateman ; Magnus Ihse Bursie 
; build-dev ; 
core-libs-...@openjdk.java.net; nd 
Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32



On Tue, Nov 27, 2018 at 7:25 PM, Nick Gasson 
mailto:nick.gas...@arm.com>> wrote:
> I missed one thing then looking at this. In TimeZone_md.c it should be
> #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> I need to change this one line before pushing.

Hi Alan,

Yes feel free to modify it. I think looking at the at other files
with these #defines more closely, is it the case that the #ifndef
MACOSX check is only required for statvfs64? As in
e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
this:

#if defined(_ALLBSD_SOURCE)
  #define stat64 stat
  #define lstat64 lstat
  #define fstat64 fstat
#endif

I don't have access to any OS X machines to test unfortunately.

But I wonder if a better way to handle this is to check for the
presence of the stat64 functions in the configure script, and
then we could just write something like this, which would be a
lot clearer:

#if !defined(HAVE_STAT64)
  #define stat64 stat
#endif


The best way is to implement

https://bugs.openjdk.java.net/browse/JDK-8165620
"Entire JDK should be built with -D_FILE_OFFSET_BITS=64"
but yes, another good way is to do as you suggest, have configure define 
HAVE_ for all known functions with a 64-bit variant and then put them into 
a header file with proper ifdefs for platforms that don't have them.

You could also "simply" add
#define _FILE_OFFSET_BITS 64
but you have to do it for cliques of files that share ambiguously sized data 
simultaneously.

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-27 Thread Martin Buchholz 
On Tue, Nov 27, 2018 at 7:25 PM, Nick Gasson  wrote:

> > I missed one thing then looking at this. In TimeZone_md.c it should be
> > #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> > I need to change this one line before pushing.
>
> Hi Alan,
>
> Yes feel free to modify it. I think looking at the at other files
> with these #defines more closely, is it the case that the #ifndef
> MACOSX check is only required for statvfs64? As in
> e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
> this:
>
> #if defined(_ALLBSD_SOURCE)
>   #define stat64 stat
>   #define lstat64 lstat
>   #define fstat64 fstat
> #endif
>
> I don't have access to any OS X machines to test unfortunately.
>
> But I wonder if a better way to handle this is to check for the
> presence of the stat64 functions in the configure script, and
> then we could just write something like this, which would be a
> lot clearer:
>
> #if !defined(HAVE_STAT64)
>   #define stat64 stat
> #endif
>


The best way is to implement

https://bugs.openjdk.java.net/browse/JDK-8165620
"Entire JDK should be built with -D_FILE_OFFSET_BITS=64"

but yes, another good way is to do as you suggest, have configure define
HAVE_ for all known functions with a 64-bit variant and then put them
into a header file with proper ifdefs for platforms that don't have them.

You could also "simply" add
#define _FILE_OFFSET_BITS 64
but you have to do it for cliques of files that share ambiguously sized
data simultaneously.

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-27 Thread Nick Gasson 
> I missed one thing then looking at this. In TimeZone_md.c it should be
> #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> I need to change this one line before pushing.

Hi Alan,

Yes feel free to modify it. I think looking at the at other files
with these #defines more closely, is it the case that the #ifndef
MACOSX check is only required for statvfs64? As in
e.g. UnixNativeDispatcher.c. So TimeZone_md.c should look like
this:

#if defined(_ALLBSD_SOURCE)
  #define stat64 stat
  #define lstat64 lstat
  #define fstat64 fstat
#endif 

I don't have access to any OS X machines to test unfortunately.

But I wonder if a better way to handle this is to check for the
presence of the stat64 functions in the configure script, and
then we could just write something like this, which would be a
lot clearer:

#if !defined(HAVE_STAT64)
  #define stat64 stat
#endif

Nick

> -Original Message-
> From: Alan Bateman 
> Sent: 28 November 2018 03:14
> To: Nick Gasson ; Magnus Ihse Bursie
> ; build-dev ;
> core-libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> On 26/11/2018 09:08, Nick Gasson wrote:
> > Hi Alan,
> >
> > I've done this here:
> >
> > http://cr.openjdk.java.net/~njian/8214077/webrev.3/
> >
> I missed one thing then looking at this. In TimeZone_md.c it should be
> #ifdef MACOSX rather than #ifndef. I can sponsor the change for you but
> I need to change this one line before pushing.
> 
> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-27 Thread Alan Bateman 

On 26/11/2018 09:08, Nick Gasson wrote:

Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.3/

I missed one thing then looking at this. In TimeZone_md.c it should be 
#ifdef MACOSX rather than #ifndef. I can sponsor the change for you but 
I need to change this one line before pushing.


-Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-27 Thread Alan Bateman 

On 27/11/2018 13:28, Magnus Ihse Bursie wrote:

26 nov. 2018 kl. 13:27 skrev Alan Bateman :


On 26/11/2018 09:08, Nick Gasson wrote:
Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.3/

This looks good and I think means we no longer have any stat usages in libjava.

Do we have stat usages in other native libraries, or are the entire code base 
in the clear now?

There are a few in the launcher that need to be examined and a few 
remaining issues in libawt_xawt and libjsound (the latter is Solaris 
specific code where I wouldn't expect anyone to be targeting 32-bit 
these days). Beyond stat then usages of open should also be examined. I 
suspect Nick is working on this incrementally.


-Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-27 Thread Magnus Ihse Bursie 


> 26 nov. 2018 kl. 13:27 skrev Alan Bateman :
> 
>> On 26/11/2018 09:08, Nick Gasson wrote:
>> Hi Alan,
>> 
>> I've done this here:
>> 
>> http://cr.openjdk.java.net/~njian/8214077/webrev.3/
> This looks good and I think means we no longer have any stat usages in 
> libjava.

Do we have stat usages in other native libraries, or are the entire code base 
in the clear now?

/Magnus

> 
> -Alan
>

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-26 Thread Alan Bateman 

On 26/11/2018 09:08, Nick Gasson wrote:

Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.3/

This looks good and I think means we no longer have any stat usages in 
libjava.


-Alan

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-26 Thread Nick Gasson 
> If you can then it would be great, if only to save others from looking
> at it and wondering if it should also be changed. Maybe for another
> issue but there are several other usages in the java launcher that
> should be looked at.

Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.3/

Thanks,
Nick

> -Original Message-
> From: Alan Bateman 
> Sent: 23 November 2018 19:42
> To: Nick Gasson ; Magnus Ihse Bursie
> ; build-dev ;
> core-libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> On 23/11/2018 11:13, Nick Gasson wrote:
> >> This looks right and reduces the stat usages in libjava down to one
> >> remaining case (ProcessHandlerImpl_linux.c).
> > Hi Alan,
> >
> > Do you want me to do this one too for completeness? Although as
> > it's calling stat() on the /proc/ directory I don't think it's 
> > possible to
> > hit the EOVERFLOW case.
> If you can then it would be great, if only to save others from looking
> at it and wondering if it should also be changed. Maybe for another
> issue but there are several other usages in the java launcher that
> should be looked at.
> 
> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-23 Thread Alan Bateman 

On 23/11/2018 11:13, Nick Gasson wrote:

This looks right and reduces the stat usages in libjava down to one
remaining case (ProcessHandlerImpl_linux.c).

Hi Alan,

Do you want me to do this one too for completeness? Although as
it's calling stat() on the /proc/ directory I don't think it's possible to
hit the EOVERFLOW case.
If you can then it would be great, if only to save others from looking 
at it and wondering if it should also be changed. Maybe for another 
issue but there are several other usages in the java launcher that 
should be looked at.


-Alan

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-23 Thread Nick Gasson 
> This looks right and reduces the stat usages in libjava down to one
> remaining case (ProcessHandlerImpl_linux.c).

Hi Alan,

Do you want me to do this one too for completeness? Although as
it's calling stat() on the /proc/ directory I don't think it's possible to
hit the EOVERFLOW case.

Thanks,
Nick

> -Original Message-
> From: Alan Bateman 
> Sent: 23 November 2018 19:01
> To: Nick Gasson ; Magnus Ihse Bursie
> ; build-dev ;
> core-libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> On 23/11/2018 09:37, Nick Gasson wrote:
> > Hi Alan,
> >
> > I've done this here:
> >
> > http://cr.openjdk.java.net/~njian/8214077/webrev.2/
> This looks right and reduces the stat usages in libjava down to one
> remaining case (ProcessHandlerImpl_linux.c).
> 
> >
> > I'm a bit unsure about #ifndef MACOSX - some existing files guard
> > the stat64 #define with this (e.g. UnixFileSystem_md.c) and some
> > don't (e.g. libnio/ch/FileDispatcherImpl.c). I don't have access
> > to an OS X machine to test, but I guess it doesn't matter too
> > much as AFAIK Apple haven't supported 32-bit for a long time?
> >
> Some sources are using _ALLBSD_SOURCE, I think this dates back to the
> Apple/BSD patch (went into 7u4 and 8). So there is a bit of consistency
> that should be cleaned up some time. Apple may have had a 32-bit build
> of the JDK many years ago but the changes that were into OpenJDK were
> for building on 64-bit only.
> 
> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-23 Thread Alan Bateman 

On 23/11/2018 09:37, Nick Gasson wrote:

Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.2/
This looks right and reduces the stat usages in libjava down to one 
remaining case (ProcessHandlerImpl_linux.c).




I'm a bit unsure about #ifndef MACOSX - some existing files guard
the stat64 #define with this (e.g. UnixFileSystem_md.c) and some
don't (e.g. libnio/ch/FileDispatcherImpl.c). I don't have access
to an OS X machine to test, but I guess it doesn't matter too
much as AFAIK Apple haven't supported 32-bit for a long time?

Some sources are using _ALLBSD_SOURCE, I think this dates back to the 
Apple/BSD patch (went into 7u4 and 8). So there is a bit of consistency 
that should be cleaned up some time. Apple may have had a 32-bit build 
of the JDK many years ago but the changes that were into OpenJDK were 
for building on 64-bit only.


-Alan

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-23 Thread Nick Gasson 
> Can you fix the stat usages in TimeZonone_md.c too?

Hi Alan,

I've done this here:

http://cr.openjdk.java.net/~njian/8214077/webrev.2/

I'm a bit unsure about #ifndef MACOSX - some existing files guard
the stat64 #define with this (e.g. UnixFileSystem_md.c) and some
don't (e.g. libnio/ch/FileDispatcherImpl.c). I don't have access
to an OS X machine to test, but I guess it doesn't matter too
much as AFAIK Apple haven't supported 32-bit for a long time?
 
Thanks,
Nick

> -Original Message-
> From: Alan Bateman 
> Sent: 22 November 2018 19:05
> To: Nick Gasson ; Magnus Ihse Bursie
> ; build-dev ;
> core-libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> 
> 
> On 22/11/2018 10:54, Nick Gasson wrote:
> >>>> Have you looked at replacing the remaining usages of stat changed to
> >>>> stat64 instead?
> >>> I've tried this just now and it also resolves the issue. I can
> >>> test on some more platforms and update the webrev if this is the
> >>> preferred solution?
> >> I'd say this is preferred to adding compiler flags, yes. The code will
> >> make it unambiguously clear that it's correct.
> > Here is the updated webrev that uses stat64:
> >
> > http://cr.openjdk.java.net/~njian/8214077/webrev.1/
> >
> Can you fix the stat usages in TimeZonone_md.c too?
> 
> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-22 Thread Alan Bateman 




On 22/11/2018 10:54, Nick Gasson wrote:

Have you looked at replacing the remaining usages of stat changed to
stat64 instead?

I've tried this just now and it also resolves the issue. I can
test on some more platforms and update the webrev if this is the
preferred solution?

I'd say this is preferred to adding compiler flags, yes. The code will
make it unambiguously clear that it's correct.

Here is the updated webrev that uses stat64:

http://cr.openjdk.java.net/~njian/8214077/webrev.1/


Can you fix the stat usages in TimeZonone_md.c too?

-Alan

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-22 Thread Nick Gasson 
> >> Have you looked at replacing the remaining usages of stat changed to
> >> stat64 instead?
> > I've tried this just now and it also resolves the issue. I can
> > test on some more platforms and update the webrev if this is the
> > preferred solution?
> I'd say this is preferred to adding compiler flags, yes. The code will
> make it unambiguously clear that it's correct.

Here is the updated webrev that uses stat64:

http://cr.openjdk.java.net/~njian/8214077/webrev.1/

Thanks,
Nick

> -Original Message-
> From: Magnus Ihse Bursie 
> Sent: 21 November 2018 20:00
> To: Nick Gasson ; Alan Bateman
> ; build-dev ; core-
> libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> On 2018-11-21 11:08, Nick Gasson wrote:
> 
> > Hi Alan,
> >
> >> Have you looked at replacing the remaining usages of stat changed to
> >> stat64 instead?
> > I've tried this just now and it also resolves the issue. I can
> > test on some more platforms and update the webrev if this is the
> > preferred solution?
> I'd say this is preferred to adding compiler flags, yes. The code will
> make it unambiguously clear that it's correct.
> 
> /Magnus
> >
> > Thanks,
> > Nick
> >
> >> -Original Message-----
> >> From: Alan Bateman 
> >> Sent: Wednesday, November 21, 2018 4:55 PM
> >> To: Nick Gasson ; build-dev  >> d...@openjdk.java.net>; core-libs-...@openjdk.java.net
> >> Cc: nd 
> >> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> >> ARM32
> >>
> >> On 21/11/2018 02:46, Nick Gasson wrote:
> >>> Hi,
> >>>
> >>> Can someone please help me review this small makefile patch to
> >>> fix an issue with java.io.File::setLastModified on 32-bit
> >>> systems?
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8214077
> >>> http://cr.openjdk.java.net/~njian/8214077/webrev.0/
> >>>
> >>> If the file size is > 2GB so that the size won't fit in a signed
> >>> 32-bit off_t all stat() calls will fail with EOVERFLOW. This causes
> >>> the native method UnixFileSystem::setLastModifiedTime to fail as it
> >>> uses stat() to preserve the access time. It also causes other methods
> >>> like File::length and File::lastModified to return 0.
> >>>
> >> Have you looked at replacing the remaining usages of stat changed to
> >> stat64 instead?
> >>
> >> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-21 Thread Magnus Ihse Bursie 

On 2018-11-21 11:08, Nick Gasson wrote:


Hi Alan,


Have you looked at replacing the remaining usages of stat changed to
stat64 instead?

I've tried this just now and it also resolves the issue. I can
test on some more platforms and update the webrev if this is the
preferred solution?
I'd say this is preferred to adding compiler flags, yes. The code will 
make it unambiguously clear that it's correct.


/Magnus


Thanks,
Nick


-Original Message-
From: Alan Bateman 
Sent: Wednesday, November 21, 2018 4:55 PM
To: Nick Gasson ; build-dev ; core-libs-...@openjdk.java.net
Cc: nd 
Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
ARM32

On 21/11/2018 02:46, Nick Gasson wrote:

Hi,

Can someone please help me review this small makefile patch to
fix an issue with java.io.File::setLastModified on 32-bit
systems?

https://bugs.openjdk.java.net/browse/JDK-8214077
http://cr.openjdk.java.net/~njian/8214077/webrev.0/

If the file size is > 2GB so that the size won't fit in a signed
32-bit off_t all stat() calls will fail with EOVERFLOW. This causes
the native method UnixFileSystem::setLastModifiedTime to fail as it
uses stat() to preserve the access time. It also causes other methods
like File::length and File::lastModified to return 0.


Have you looked at replacing the remaining usages of stat changed to
stat64 instead?

-Alan

RE: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-21 Thread Nick Gasson 
Hi Alan,

> Have you looked at replacing the remaining usages of stat changed to
> stat64 instead?

I've tried this just now and it also resolves the issue. I can
test on some more platforms and update the webrev if this is the
preferred solution?

Thanks,
Nick

> -Original Message-
> From: Alan Bateman 
> Sent: Wednesday, November 21, 2018 4:55 PM
> To: Nick Gasson ; build-dev  d...@openjdk.java.net>; core-libs-...@openjdk.java.net
> Cc: nd 
> Subject: Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on
> ARM32
> 
> On 21/11/2018 02:46, Nick Gasson wrote:
> > Hi,
> >
> > Can someone please help me review this small makefile patch to
> > fix an issue with java.io.File::setLastModified on 32-bit
> > systems?
> >
> > https://bugs.openjdk.java.net/browse/JDK-8214077
> > http://cr.openjdk.java.net/~njian/8214077/webrev.0/
> >
> > If the file size is > 2GB so that the size won't fit in a signed
> > 32-bit off_t all stat() calls will fail with EOVERFLOW. This causes
> > the native method UnixFileSystem::setLastModifiedTime to fail as it
> > uses stat() to preserve the access time. It also causes other methods
> > like File::length and File::lastModified to return 0.
> >
> Have you looked at replacing the remaining usages of stat changed to
> stat64 instead?
> 
> -Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-21 Thread Alan Bateman 

On 21/11/2018 02:46, Nick Gasson wrote:

Hi,

Can someone please help me review this small makefile patch to
fix an issue with java.io.File::setLastModified on 32-bit
systems?

https://bugs.openjdk.java.net/browse/JDK-8214077
http://cr.openjdk.java.net/~njian/8214077/webrev.0/

If the file size is > 2GB so that the size won't fit in a signed
32-bit off_t all stat() calls will fail with EOVERFLOW. This causes
the native method UnixFileSystem::setLastModifiedTime to fail as it
uses stat() to preserve the access time. It also causes other methods
like File::length and File::lastModified to return 0.

Have you looked at replacing the remaining usages of stat changed to 
stat64 instead?


-Alan

Re: RFR: 8214077: test java/io/File/SetLastModified.java fails on ARM32

2018-11-20 Thread David Holmes 

Hi Nick,

I'll leave it for core-libs and build folk to review this, but just for 
some background ... This is a bit of a recurring issue. We have the all 
encompassing:


https://bugs.openjdk.java.net/browse/JDK-8165620
"Entire JDK should be built with -D_FILE_OFFSET_BITS=64"

then we had the long standing:

https://bugs.openjdk.java.net/browse/JDK-8062658
"Use of 32-bit stat on 64-bit filesystems"

which was closed as "Will not fix" due to no one owning support for 
32-bit systems at the time.


Then we have spot fixes like:

https://bugs.openjdk.java.net/browse/JDK-8156181
"UL: File size limit on 32 bit Linux"

https://bugs.openjdk.java.net/browse/JDK-8165643
"SecureDirectoryStream doesn't work on linux non-x86"

https://bugs.openjdk.java.net/browse/JDK-712
"GC log is limited to 2G for 32-bit"

So I guess another spot fix is not unacceptable, but it might be worth 
considering picking up the gauntlet for JDK-8165620 and fixing this 
across the board once and for all.


Cheers,
David

On 21/11/2018 12:46 pm, Nick Gasson wrote:

Hi,

Can someone please help me review this small makefile patch to
fix an issue with java.io.File::setLastModified on 32-bit
systems?

https://bugs.openjdk.java.net/browse/JDK-8214077
http://cr.openjdk.java.net/~njian/8214077/webrev.0/

If the file size is > 2GB so that the size won't fit in a signed
32-bit off_t all stat() calls will fail with EOVERFLOW. This causes
the native method UnixFileSystem::setLastModifiedTime to fail as it
uses stat() to preserve the access time. It also causes other methods
like File::length and File::lastModified to return 0.

Fix by defining _FILE_OFFSET_BITS=64 when building
UnixFileSystem_md.c, this will make off_t 64 bits.

Found on ARM32 but I believe this affects 32-bit x86 too. Checked for
no new Jtreg regressions with this patch.

Thanks,
Nick