Thank you very much, David, for helping file the bug in system. Hi, Core Libraries Group,
I am not quite clear who owns jdk.pack/share/native/common-unpack/zip.cpp (at jdk/jdk for example). Could anyone please help? The issue covered in this thread is a simple but a generic concern to all branches having the same declaration of "gmtime_r", and it was found with GCC 8.2 but a generic one to other versions, including the latest GCC 9. I tested the combinations such as CentOS+aarch64, Ubuntu+x86 etc., the patch works well for compilation. I have no permission to upload the webrev result to cr.openjdk.java.net, attached it << JDK-8215976.webrev.00.zip>>. If you cannot get it, see detail in https://bugs.openjdk.java.net/browse/JDK-8215976 please. Thanks for your review. Regards Patrick -----Original Message----- From: David Holmes <david.hol...@oracle.com> Sent: Wednesday, January 2, 2019 7:11 AM To: Patrick Zhang <patr...@os.amperecomputing.com>; core-libs-dev@openjdk.java.net Cc: Florian Weimer <fwei...@redhat.com> Subject: Re: OpenJDK fails to build with GCC when the #include<time.h> inside zip.cpp comes from a non-sysroot path Hi Patrick, On 13/12/2018 1:23 pm, Patrick Zhang wrote: > Ping... > > Apply for a sponsor for this simple patch. Seems no one wants to own this one :( > I doubt if I could have permission to file the issue/bug report anywhere, > could anyone kindly give a guidance? Thanks. I have filed: https://bugs.openjdk.java.net/browse/JDK-8215976 to cover this and linked to the discussion threads. The appropriate owners still need to review this, but I'm not sure who that may be these days. Cheers, David ----- > > Regards > Patrick > > -----Original Message----- > From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> On Behalf > Of Patrick Zhang > Sent: Thursday, December 6, 2018 4:28 PM > To: core-libs-dev@openjdk.java.net; David Holmes > <david.hol...@oracle.com> > Cc: Florian Weimer <fwei...@redhat.com> > Subject: RE: OpenJDK fails to build with GCC when the #include<time.h> > inside zip.cpp comes from a non-sysroot path > > To All, > Who could help sponsor this simple patch (may require a wide range of > building tests)? Thanks in advance. > > Regards > Patrick > > -----Original Message----- > From: David Holmes <david.hol...@oracle.com> > Sent: Monday, December 3, 2018 8:11 AM > To: Patrick Zhang <patr...@os.amperecomputing.com>; Florian Weimer > <fwei...@redhat.com> > Cc: core-libs-dev@openjdk.java.net > Subject: Re: OpenJDK fails to build with GCC when the #include<time.h> > inside zip.cpp comes from a non-sysroot path > > Hi Patrick, > > On 30/11/2018 11:41 pm, Patrick Zhang wrote: >> Thanks Florian, the "-isystem /usr/include" is helpful to my case, I see >> gcc.gnu.org says that "it gets the same special treatment that is applied to >> the standard system directories". As such the issue gets hidden (error >> suppressed). >> >> Hi David, >> Thanks for your suggestion. My intention was to limit the influence range as >> far as I could since I don't have other systems except CentOS/Fedora to >> verify (even just smoke test) all paths. > > You'd need some assistance testing a wider range of platforms but that can be > arranged. > >> In addition, if we make below update, does it mean the macro " _REENTRANT " >> can be removed too? This is probably the only place where _REENTRANT gets >> used AFAIK. > _REENTRANT is also examined by system headers. It's probably legacy but would > require more investigation. > > Cheers, > David > >> #ifdef _MSC_VER // Windows >> #define gmtime_r(t, s) gmtime(t) >> #endif >> >> Regards >> Patrick >> >> -----Original Message----- >> From: Florian Weimer <fwei...@redhat.com> >> Sent: Thursday, November 29, 2018 8:02 PM >> To: David Holmes <david.hol...@oracle.com> >> Cc: Patrick Zhang <patr...@os.amperecomputing.com>; >> jdk-...@openjdk.java.net; core-libs-dev@openjdk.java.net >> Subject: Re: OpenJDK fails to build with GCC when the >> #include<time.h> inside zip.cpp comes from a non-sysroot path >> >> * David Holmes: >> >>> This should really be being discussed on core-libs-dev. >> >> Okay, moving the conversation. >> >>>> diff -r 70a423caee44 src/share/native/com/sun/java/util/jar/pack/zip.cpp >>>> --- a/src/share/native/com/sun/java/util/jar/pack/zip.cpp Tue Oct 09 >>>> 08:33:33 2018 +0100 >>>> +++ b/src/share/native/com/sun/java/util/jar/pack/zip.cpp Wed Nov 28 >>>> 22:13:12 2018 -0500 >>>> @@ -415,9 +415,7 @@ >>>> ((uLong)h << 11) | ((uLong)m << 5) | ((uLong)s >> 1); >>>> } >>>> -#ifdef _REENTRANT // solaris >>>> -extern "C" struct tm *gmtime_r(const time_t *, struct tm *); >>>> -#else >>>> +#if !defined(_REENTRANT) // linux >>>> #define gmtime_r(t, s) gmtime(t) >>>> #endif >>>> /* >>> >>> Under the theme "two wrongs don't make a right" the use of >>> _REENTRANT here is totally inappropriate AFAICS. It seems to be a >>> misguided attempt at determining whether we need the thread-safe >>> gmtime_r or not >>> - and the answer to that should always be yes IMHO. >>> >>> We define _REENTRANT for: >>> - linux >>> - gcc >>> - xlc >>> >>> So the original code will define: >>> >>> extern "C" struct tm *gmtime_r(const time_t *, struct tm *); >>> >>> for linux (and AIX with xlc?) but not Solaris, OS X or Windows. >>> >>> But Solaris has gmtime_r anyway. So the existing code seems a really >>> convoluted hack. AFAICS we have gmtime_r everywhere but Windows >>> (where gmtime is already thread-safe). So it seems to me that all we >>> should need here is: >>> >>> -#ifdef _REENTRANT // solaris >>> -extern "C" struct tm *gmtime_r(const time_t *, struct tm *); -#else >>> +#ifdef _MSC_VER // Windows >>> #define gmtime_r(t, s) gmtime(t) >>> #endif >> >> That looks much cleaner. >> >> Thanks, >> Florian >>
