RFR(XXS): 8132232: Signature mismatch between declaration and definition of PosixSemaphore::timedwait

[David Holmes]

Hi Volker,

This change seems fine to me. I suspect Kim originally suggested the 
"const" but it only got applied in one place. Adding "const" would 
likely have a flow on effect to the create_timespec function so I'm okay 
with just removing it. But I think Kim should have a say :)

Thanks,
David

On 24/07/2015 2:01 AM, Volker Simonis wrote:
> On Thu, Jul 23, 2015 at 4:39 PM, Volker Simonis
> <volker.simonis at gmail.com> wrote:
>> Hi,
>>
>> can somebody please review and sponsor this tiny change to fix the build on
>> Solaris/Sparc with SS12u3:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/2015/8132232
>> https://bugs.openjdk.java.net/browse/JDK-8132232
>>
>> In semaphore_posix.hpp timedwait is declared as follows:
>>
>> class PosixSemaphore : public CHeapObj<mtInternal> {
>>   private:
>>    bool timedwait(struct timespec ts);
>> }
>>
>> but in os_posix.cpp it is defined as follows:
>>
>> bool PosixSemaphore::timedwait(const struct timespec ts) {
>>
>> On Solaris 10/11 on Sparc with SS12u3 (Sun C++ 5.12 SunOS_sparc 2011/11/16)
>> this gives an error in the release build:
>>
>> Undefined first referenced
>>   symbol in file
>> bool PosixSemaphore::timedwait(timespec) os_solaris.o
>>
>> This is because the caller in os_solaris.o requires:
>>
>> /usr/ccs/bin/nm -C hotspot/solaris_sparcv9_compiler2/product/os_solaris.o |
>> grep timedwait
>> [456] | 0| 0|FUNC |GLOB |0 |UNDEF |bool PosixSemaphore::timedwait(timespec)
>> [__1cOPosixSemaphoreJtimedwait6MnItimespec__b_]
>>
>> but the implementation in os_posix.o has:
>>
>> /usr/ccs/bin/nm -C hotspot/solaris_sparcv9_compiler2/product/os_posix.o |
>> grep timedwait
>> [61] | 6928| 124|FUNC |GLOB |0 |2 |bool PosixSemaphore::timedwait(const
>> timespec)
>> [__1cOPosixSemaphoreJtimedwait6MknItimespec__b_]
>>
>> Strange enough, the error doesn't seem to happen on Solaris/AMD64 (using the
>> exactly same compiler version) and I absolutely can not see how this error
>> is related to the CPU architecture!
>>
>
> So this is definitely a compiler bug. The compiler should treat
> functions with "Parameter declarations that differ only in the
> presence or absence of const and/or volatile as equivalent" (C++03
> 13.1-3, see for example
> http://stackoverflow.com/questions/3681188/why-does-a-function-declaration-with-a-const-argument-allow-calling-of-a-function).
>
> Nevertheless I think we should fix this because the fix is trivial and
> I still think it is good style to use the same signature for both, the
> definition and the declaration of a function, no matter whether the
> compiler treats them as unique or not.
>
>> I also can not understand why nobody has seen this before? Maybe you
>> (Oracle) are using a newer compiler where this is fixed? But the "Supported
>> Build Platforms" wiki page [1] still mentions 12.3.
>>
>> Fortunately, the fix is trivial - just remove the const qualifier from the
>> method definition.
>>
>> Regards,
>> Volker
>>
>> [1] https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms