RFR: Handle confstr version lookup failures gracefully

[Mikael Vidstedt]

> On Apr 11, 2017, at 7:43 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> 
> 
> On 12/04/2017 12:06 PM, Mikael Vidstedt wrote:
>> 
>>> On Apr 11, 2017, at 6:58 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>> <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>>> wrote:
>>> 
>>> On 12/04/2017 11:42 AM, Mikael Vidstedt wrote:
>>>> 
>>>>> On Apr 11, 2017, at 6:27 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>
>>>>> <mailto:david.holmes at oracle.com <mailto:david.holmes at oracle.com>>> wrote:
>>>>> 
>>>>> On 12/04/2017 10:00 AM, Mikael Vidstedt wrote:
>>>>>> 
>>>>>> During bootstrapping, os::Linux::libpthread_init() tries to
>>>>>> discover the versions of (g)libc and libpthread respectively. This
>>>>>> is done using confstr(3). However, musl doesn’t implement support
>>>>>> for version discovery, so confstr will return 0/EINVAL.
>>>>>> 
>>>>>> This change makes the code handle confstr failures gracefully, and
>>>>>> defaults using the string “unknown” if the version information
>>>>>> cannot be looked up.
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~mikael/webrevs/portola/glibcversion/webrev.01/hotspot/webrev/
>>>>> 
>>>>> There was no need to separate out the declaration here:
>>>>> 
>>>>> 504   size_t n;
>>>>> 505
>>>>> 506   n = confstr(_CS_GNU_LIBC_VERSION, NULL, 0);
>>>>> 
>>>>> Overall I don't think this change is quite what we would want in
>>>>> that it is wrong to print "unknown" when in fact there is no libc
>>>>> (or libpthread?) involved. And we would surely want to see that we
>>>>> are using musl.
>>>> 
>>>> Since the variable is reused for the second confstr call I prefer to
>>>> declare it separately.
>>> 
>>> Declaring uninitialized variables is an anachronism IMHO. :)
>>> 
>>>> In an upcoming change I have added support for adding “-musl” to the
>>>> (internal) version string so that we can at least see that it’s a
>>>> musl VM instead of a glibc one.
>>>> 
>>>> There is a libc - musl. libpthread happens to be part of that same
>>>> libc instead of being a separate library, so depending on how you
>>>> look at it there is or isn’t a libpthread (it is effectively an
>>>> implementation detail of musl). The problem is that there’s no way of
>>>> getting the version information at runtime with musl...
>>> 
>>> So if we can detect that we are musl then the libc version should be
>>> "musl" and the libpthread version should be "<integrated>" (or
>>> something like that.
>> 
>> That’s fair. My upcoming change will pass in the target C library as an
>> explicit compiler define. Detecting it at runtime is AFAICT not
>> possible. Once I have the change in place I can go back and update the
>> values of these two “versions”.
>> 
>>> I'm surprised there is no mechanism at all for runtime detection of musl!
>>> 
>>> But musl defines in unistd.h
>>> 
>>> #define _CS_GNU_LIBC_VERSION2
>>> #define _CS_GNU_LIBPTHREAD_VERSION3
>>> 
>>> so won't those be reported by confstr ??
>> 
>> Unfortunately not. confstr will return 0/EINVAL for almost all
>> arguments. Those two defines are likely only there to make code compile
>> at all with musl, but if confstr is actually called at runtime with any
>> of those values it will return an error.
> 
> I can not make any sense out of:
> 
> https://git.musl-libc.org/cgit/musl/tree/src/conf/confstr.c <https://git.musl-libc.org/cgit/musl/tree/src/conf/confstr.c>
> 
> Quality code - love the clear explanatory comments. :(

I’ve stared at that one before wondering what the second if statement is trying to express, but in either case it’s not going to return anything useful :)

Cheers,
Mikael

> 
> David
> 
>> 
>> Cheers,
>> Mikael
>> 
>>> 
>>> Thanks,
>>> David
>>> 
>>>> Cheers,
>>>> Mikael