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