RFR: 8295146: Clean up native code with newer C/C++ language features [v3]

[Kim Barrett]

On Wed, 23 Nov 2022 05:22:10 GMT, Kim Barrett <kbarrett at openjdk.org> 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