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

[Volker Simonis]

Hi David,

thanks for looking into this issue.

Just to clarify: according to the standard this const has no effect at
all and should be ignored by the compiler (i.e. foo(int) and foo(const
int) is the same). I just want to adjust the two occurrences in the
declaration and the definition (preferably by removing const because
it doesn't has any effect anyway) to work around a compiler bug AND
for code style reasons to prevent further ambiguities.

Regards,
Volker


On Fri, Jul 24, 2015 at 8:56 AM, David Holmes <david.holmes at oracle.com> wrote:
> 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