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

[David Holmes]

On 2/06/2017 3:54 PM, Kim Barrett wrote:
>> On Jun 2, 2017, at 12:55 AM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8181451
>> webrev: http://cr.openjdk.java.net/~dholmes/8181451/webrev/
> 
> Looks good.

Thanks Kim! Again many thanks for determining the underlying problem.

David

>>
>> 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
> 
>