On Wed, 23 Nov 2022 05:22:10 GMT, Kim Barrett <[email protected]> wrote:
>> It's to avoid redefining the linkage as static in os_windows.cpp (where it's
>> implemented) after an extern declaration (inside the class), which is
>> forbidden by C++11:
>>
>>> The linkages implied by successive declarations for a given entity shall
>>> agree. That is, within a given scope, each declaration declaring the same
>>> variable name or the same overloading of a function name shall imply the
>>> same linkage.
>>
>> While 2019 by default seems to ignore this rule and accepts the conflicting
>> linkage as a language extension, this can cause issues with newer and
>> stricter versions of the Visual C++ compiler (especially with -permissive-
>> passed during compilation, which Magnus and Daniel have pointed out in
>> another discussion will become the default mode of compilation in the
>> future). It's not possible to declare a static friend inside a class, so the
>> addition above takes advantage of another C++ feature instead:
>>
>>> ยง11.3/4 [class.friend]
>> A function first declared in a friend declaration has external linkage
>> (3.5). Otherwise, the function retains its previous linkage (7.1.1).
>
> I think the problem here is the friend declaration, which doesn't look like
> it's needed and could be deleted.
Digging into this some more, the friend declaration exists to provide access to
the private `os::win32::enum Ept`.
One obvious and cheap solution to that would be to make that enum public. I
think that would be an improvement vs the current friend declaration. But
there are some other things one could complain about there, such as the type of
the function requiring a complicated function pointer cast where it's used.
Here's a patch that I think cleans this up.
diff --git a/src/hotspot/os/windows/os_windows.cpp
b/src/hotspot/os/windows/os_windows.cpp
index 0651f0868f3..bf9e759b1d6 100644
--- a/src/hotspot/os/windows/os_windows.cpp
+++ b/src/hotspot/os/windows/os_windows.cpp
@@ -511,7 +511,9 @@ JNIEXPORT
LONG WINAPI topLevelExceptionFilter(struct _EXCEPTION_POINTERS* exceptionInfo);
// Thread start routine for all newly created threads
-static unsigned __stdcall thread_native_entry(Thread* thread) {
+// Called with the associated Thread* as the argument.
+unsigned __stdcall os::win32::thread_native_entry(void* t) {
+ Thread* thread = static_cast<Thread*>(t);
thread->record_stack_base_and_size();
thread->initialize_thread_current();
@@ -744,7 +746,7 @@ bool os::create_thread(Thread* thread, ThreadType thr_type,
thread_handle =
(HANDLE)_beginthreadex(NULL,
(unsigned)stack_size,
- (unsigned (__stdcall *)(void*))
thread_native_entry,
+ &os::win32::thread_native_entry,
thread,
initflag,
&thread_id);
diff --git a/src/hotspot/os/windows/os_windows.hpp
b/src/hotspot/os/windows/os_windows.hpp
index 94d7c3c5e2d..197797078d7 100644
--- a/src/hotspot/os/windows/os_windows.hpp
+++ b/src/hotspot/os/windows/os_windows.hpp
@@ -36,7 +36,6 @@ typedef void (*signal_handler_t)(int);
class os::win32 {
friend class os;
- friend unsigned __stdcall thread_native_entry(Thread*);
protected:
static int _processor_type;
@@ -70,6 +69,10 @@ class os::win32 {
static HINSTANCE load_Windows_dll(const char* name, char *ebuf, int ebuflen);
private:
+ // The handler passed to _beginthreadex().
+ // Called with the associated Thread* as the argument.
+ static unsigned __stdcall thread_native_entry(void*);
+
enum Ept { EPT_THREAD, EPT_PROCESS, EPT_PROCESS_DIE };
// Wrapper around _endthreadex(), exit() and _exit()
static int exit_process_or_thread(Ept what, int exit_code);
-------------
PR: https://git.openjdk.org/jdk/pull/11081
