RFR (S): 8151322: Implement os::set_native_thread_name() on Solaris

[David Holmes]

On 22/03/2016 1:21 PM, Kim Barrett wrote:
>> On Mar 21, 2016, at 8:00 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> reinterpret_cast (and all the C-style casts involved are really
>>> reinterpret_casts) can be used to convert a function pointer to a
>>> different function pointer type.  It can also be used to convert
>>> between any pointer (including function pointers) and (appropriate)
>>> integer types.  No other conversions involving function pointers are
>>> allowed.  (C++11 adds "Converting a function pointer to an object
>>> pointer type or vice versa is conditionally-supported.")
>>>
>>> Since void* is not a function pointer type, there's no specified
>>> direct conversion from a void* to a function pointer.  That's the
>>> raison d'etre for CAST_TO_FN_PTR. (Although conversion to an integer
>>> then to some other type (not back to the original type) and then use
>>> is implementation-defined.)
>>>
>>> However, it seems that some compilers (including the Solaris versions
>>> we've been using) allow direct void* -> function pointer conversions.
>>
>> I thought any pointer type could be converted to void* and back again?
>
> Only for object pointers, not for function pointers.

Ah!

> Note, however, that the POSIX description of dlsym says (in the non-normative “Application Usage” section):
>
>      Note that conversion from a void * pointer to a function pointer as in:
>
>      fptr = (int (*)(int))dlsym(handle, "my_function");
>
>      is not defined by the ISO C standard. This standard requires this conversion to work correctly on conforming implementations.
>
> I only noticed this today.  So maybe CAST_TO_FN_PTR is not needed in Unix/POSIX-specific code?

You pre-empted my next question: given dlsym returns void* and has to be 
converted to a function ptr there would have to be some legal way to 
achieve this. :) Kind of interesting though that POSIX puts an 
obligation on the compiler writer.

Looking at the C standard, given any function-ptr can be converted to 
another type of function-ptr and back without change, it seems to me 
that the return type of dlsym (and related library calls) should be 
void(*)(void) to be strictly correct wrt the language standards. But in 
practice as long as pointers are all the same size (which seems the 
obvious and common implementation [ but by no means guaranteed ]) then 
the conversion works correctly with no actual changes to the bits.

> Unfortunately, the description of CAST_TO_FN_PTR is kind of vague, so it’s unclear who the culprit(s) that led to it might be.  It would be easy enough to simplify the macro definition and see if any currently supported compilers complain.  And there are only 136 occurrences to clean up if that becomes a thing to do.

The comment seems clear enough, though without naming problematic compilers:

//
// ANSI C++ does not allow casting from one pointer type to a function 
pointer
// directly without at best a warning. This macro accomplishes it silently
// In every case that is present at this point the value be cast is a 
pointer
// to a C linkage function. In some case the type used for the cast reflects
// that linkage and a picky compiler would not complain. In other cases 
because
// there is no convenient place to place a typedef with extern C linkage 
(i.e
// a platform dependent header file) it doesn't. At this point no 
compiler seems
// picky enough to catch these instances (which are few). It is possible 
that
// using templates could fix these for all cases. This use of templates 
is likely
// so far from the middle of the road that it is likely to be problematic in
// many C++ compilers.
//

but that dates back to pre-1999 so ... :)

In any case it seems CAST_TO_FN_PTR is really for shared code and in 
platform specific code we should know whether supported compilers have 
an issue or not. Of course it would have been better to have been 
consistent about this throughout the codebase, but when writing 
platform-specific code it is rarely obvious when something like 
CAST_TO_FN_PTR is needed, if the "obvious" coding pattern works.

Just for fun I'll do a JPRT run to see if anything complains if 
CAST_TO_FN_PTR(type,val) simply does ((type)(val)).

Cheers,
David