On Wed, 5 Jul 2023 06:40:11 GMT, Daniel Jeliński <djelin...@openjdk.org> wrote:

>> Please review this test-only fix that fixes the size of variables that are 
>> used in methods that expect a pointer.
>> 
>> On Windows, type `long` is 32 bits, pointers are 64 bits large. The method 
>> `GetThreadLocalStorage` writes a pointer (64 bits) to the address given by 
>> its parameter, which overflows a `long`. The code generated by VS compiler 
>> ignores this, but the code generated by clang crashes the test.
>> 
>> No new tests. MethodExitTest continues to pass on supported platforms, and 
>> passes on clang+win with this fix.
>
> Daniel Jeliński has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Use void* instead

Looks good albeit more extensive than I had envisaged.

Thanks.

test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp
 line 127:

> 125: static void* tls_data = 0;
> 126: static const void* const tls_data1 = (const void*)0x111;
> 127: static const void* const tls_data2 = (const void*)0x222;

Was this necessitated by the change to `void*`? If so I had not expected that.

-------------

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14770#pullrequestreview-1515895320
PR Review Comment: https://git.openjdk.org/jdk/pull/14770#discussion_r1254025645

Reply via email to