RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS

[David Holmes]

On 21/02/2020 2:22 am, Reingruber, Richard wrote:
>    >
>    > I wonder why the VM doesn't use gnu_get_libc_version() ? os_linux.cpp does:
>    >
>    >    size_t n = confstr(_CS_GNU_LIBC_VERSION, NULL, 0);
>    >    assert(n > 0, "cannot retrieve glibc version");
>    >    char *str = (char *)malloc(n, mtInternal);
>    >    confstr(_CS_GNU_LIBC_VERSION, str, n);
> 
> It used to: https://bugs.openjdk.java.net/browse/JDK-8078513
> 
> Personally I would apply
> 
> -  size_t n = confstr(_CS_GNU_LIBC_VERSION, NULL, 0);
> -  assert(n > 0, "cannot retrieve glibc version");
> -  char *str = (char *)malloc(n, mtInternal);
> -  confstr(_CS_GNU_LIBC_VERSION, str, n);
> -  os::Linux::set_glibc_version(str);
> -
> -  n = confstr(_CS_GNU_LIBPTHREAD_VERSION, NULL, 0);
> +  os::Linux::set_glibc_version(gnu_get_libc_version());

Thomas obviously thinks confstr is superior for some reason. :)

Cheers,
David

> 
>    > I'm okay with the test changes.
> 
> Thanks!
> Richard.
> 
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Donnerstag, 20. Februar 2020 10:34
> To: Reingruber, Richard <richard.reingruber at sap.com>; Jiangli Zhou <jianglizhou at google.com>; hotspot-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS
> 
> On 20/02/2020 7:03 pm, Reingruber, Richard wrote:
>> Hi Jiangli,
>>
>> thanks for providing feedback.
>>
>>       > David's suggestion of using dlsym seems to be more reliable. Regarding the stability of
>>       > __pthread_get_minstack in future glibc versions, Florian has added comment to
>>       > __pthread_get_minstack about external users to ensure it's not removed until TCB is
>>       > separately allocated.  Please see: http://51.15.138.76/patch/18989/ and
>>       > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=91628#c2.
>>
>> So if I understand you and David correctly, then the test should be skipped if the lookup of
>> __pthread_get_minstack fails.
>>
>> Then the test would be skipped on future glibc versions where __pthread_get_minstack was removed
>> because TLS and TCB are not placed on a threads stack anymore.
> 
> I don't think that is really a significant concern. As Jiangli pointed
> out any change will be communicated beforehand and we will adjust the VM
> code and the test as needed. But okay ...
> 
> I wonder why the VM doesn't use gnu_get_libc_version() ? os_linux.cpp does:
> 
>     size_t n = confstr(_CS_GNU_LIBC_VERSION, NULL, 0);
>     assert(n > 0, "cannot retrieve glibc version");
>     char *str = (char *)malloc(n, mtInternal);
>     confstr(_CS_GNU_LIBC_VERSION, str, n);
> 
> I'm okay with the test changes.
> 
> Thanks,
> David
> -----
> 
>> Actually I would prefer not to skip the test then. I'd like to verify if large data structures can
>> be placed in TLS without stack size adjustment or if there is some kind of regression. That's the
>> point of regression tests after all.
>>
>> In other words: I wouldn't change test coverage of future glibc versions. I just want to exclude
>> glibc versions were the tested feature cannot work.
>>
>> Thanks, Richard.
>>
>> -----Original Message-----
>> From: Jiangli Zhou <jianglizhou at google.com>
>> Sent: Mittwoch, 19. Februar 2020 19:24
>> To: Reingruber, Richard <richard.reingruber at sap.com>
>> Cc: hotspot-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS
>>
>> Hi Richard,
>>
>> David's suggestion of using dlsym seems to be more reliable. Regarding
>> the stability of __pthread_get_minstack in future glibc versions,
>> Florian has added comment to __pthread_get_minstack about external
>> users to ensure it's not removed until TCB is separately allocated.
>> Please see: http://51.15.138.76/patch/18989/ and
>> https://gcc.gnu.org/bugzilla//show_bug.cgi?id=91628#c2.
>>
>> Best regards,
>> Jiangli
>>
>>
>>
>>
>> On Wed, Feb 19, 2020 at 2:44 AM Reingruber, Richard
>> <richard.reingruber at sap.com> wrote:
>>>
>>> Hi,
>>>
>>> please review this small test fix
>>>
>>> Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8239449/webrev.0/
>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8239449
>>>
>>> On linux systems with older glibc versions (e.g. 2.11) the option -XX:+AdjustStackSizeForTLS has no effect, because the private function __pthread_get_minstack is not provided, and the test fails. With the fix the test is skipped instead.
>>>
>>> Thanks, Richard.