OpenJDK fails to build with GCC when the #include<time.h> inside zip.cpp comes from a non-sysroot path

Patrick Zhang patrick at os.amperecomputing.com
Tue Jan 15 08:57:13 UTC 2019


Hi Roger,

Thanks for your review firstly.
Attached is the latest patch rebased on today's tip of jdk/jdk (http://hg.openjdk.java.net/jdk/jdk/rev/0b2574a2a6c7). 
<<JDK-8215976.webrev.01.zip>>

Yes there is a "#ifndef _MSC_VER" at line 36, while I think we'd better keep this "#ifdef _MSC_VER" here, as such the declaration of gmtime_r can be near to where it gets used (line 436 in the patch).

FYI. I sent out an almost same patch early on 3 Jan.
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-January/057679.html 
<<JDK-8215976.webrev.00.zip>>

Regards
Patrick

From: Roger Riggs <Roger.Riggs at oracle.com> 
Sent: Monday, January 14, 2019 11:49 PM
To: Patrick Zhang <patrick at os.amperecomputing.com>
Cc: core-libs-dev <core-libs-dev at 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,

Please re-post the entire proposed patch based on the JDK 13 repo.

BTW, there is already a "#ifndef _MSC_VER" at line 36.

Thanks, Roger

On 01/14/2019 09:02 AM, Magnus Ihse Bursie wrote:
On 2019-01-02 00:11, David Holmes wrote: 

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 :( 
For what it's worth, it looks good to me. (But I'd like to see a core library developer chime in as well.) 

/Magnus 




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 mailto:core-libs-dev-bounces at openjdk.java.net On Behalf Of Patrick Zhang 
Sent: Thursday, December 6, 2018 4:28 PM 
To: mailto:core-libs-dev at openjdk.java.net; David Holmes mailto:david.holmes at oracle.com 
Cc: Florian Weimer mailto:fweimer at 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 mailto:david.holmes at oracle.com 
Sent: Monday, December 3, 2018 8:11 AM 
To: Patrick Zhang mailto:patrick at os.amperecomputing.com; Florian Weimer mailto:fweimer at redhat.com 
Cc: mailto:core-libs-dev at 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 mailto:fweimer at redhat.com 
Sent: Thursday, November 29, 2018 8:02 PM 
To: David Holmes mailto:david.holmes at oracle.com 
Cc: Patrick Zhang mailto:patrick at os.amperecomputing.com; 
mailto:jdk-dev at openjdk.java.net; mailto:core-libs-dev at 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 




More information about the core-libs-dev mailing list