RFR (XS): 8181451: JDK-8174231 broke some clang builds

David Holmes david.holmes at oracle.com
Fri Jun 2 09:08:09 UTC 2017


Hi Thomas,

On 2/06/2017 3:49 PM, Thomas Stüfe wrote:
> Hi David,
> 
> good catch :) Change is fine.

Thanks for reviewing.

> I wonder why the macos port needed to typedef clockid_t int; in the 
> first place.

No idea.

> Also, IMHO this would be another argument for getting rid of this weird 
> inclusion scheme of the os_xxx headers sometime in the future. One never 
> thinks about that everything you put in there ends up automatically in 
> the os namespace, in the middle of os.hpp.

Yes, Kim made a similar comment. :) But I there is still uncertainty 
about the best way to restructure things.

Cheers,
David

> Kind Regards, Thomas
> 
> On Fri, Jun 2, 2017 at 6:55 AM, David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     bug: https://bugs.openjdk.java.net/browse/JDK-8181451
>     <https://bugs.openjdk.java.net/browse/JDK-8181451>
>     webrev: http://cr.openjdk.java.net/~dholmes/8181451/webrev/
>     <http://cr.openjdk.java.net/~dholmes/8181451/webrev/>
> 
>     tl;dr: simple fix, but interesting explanation :)
> 
>     The fix for 8174231 actually exposed a flaw in the os_bsd.hpp
>     header, where we have this:
> 
>     #ifdef __APPLE__
>     // Mac OS X doesn't support clock_gettime. Stub out the type, it is
>     // unused
>     typedef int clockid_t;
>     #endif
> 
>     When building for Mac OS the clockid_t is never used (ifdef
>     !_APPLE_) and so the typedef is not needed. As of Mac OS X 10.12
>     (Sierra) clock_gettime _is_ supported, but the real clockid_t is a
>     enum - hence the existing typedef is incorrect (but probably
>     harmless at runtime given enums are physically if not logically ints).
> 
>     What was going wrong after the fix for 8174231 is that the initial
>     use of clockid_t to describe the _clock_gettime_func variable was in
>     the global scope (so the enum); whereas the actual assignment to
>     that variable, using a cast, is in the os::Posix class scope, and
>     the typedef above is also in the os class scope, and so an int.
>     Hence the compiler complained that the clockid_t-enum on the left of
>     the assignment, was not the same as the clockid_t-int on the right.
> 
>     Thanks to Igor for finding this (we don't build or test on Mac OS X
>     10.12 normally) and the initial suggested fix - which used a typedef
>     for the clock_gettime function type and so ensured the types on the
>     left and right of the assignment were the same (and the enum at that!).
> 
>     Many thanks to Kim for solving the mystery of how the two
>     clockid_t's on the same line of source code could end up being
>     different types! After wading through that I really do need my
>     upcoming vacation. :)
> 
>     Oh and one final word. While OS X 10.12 now supports clock_gettime
>     and CLOCK_MONOTONIC we do not use them. The time functions still use
>     the mach time functions. There is still no support for
>     pthread_condattr_setclock, so we don't use CLOCK_MONOTONIC there
>     either. (Which is a bug waiting to happen as we want consistent use
>     of clocks!).
> 
>     Thanks,
>     David
> 
> 


More information about the hotspot-runtime-dev mailing list