RFR: 8339783: Implement JEP 479: Remove the Windows 32-bit x86 Port [v30]

[Kim Barrett]

On Mon, 4 Nov 2024 20:42:59 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

>> This is the implementation of [JEP 479: _Remove the Windows 32-bit x86 Port_](https://openjdk.org/jeps/479).
>> 
>> This is the summary of JEP 479:
>>> Remove the source code and build support for the Windows 32-bit x86 port. This port was [deprecated for removal in JDK 21](https://openjdk.org/jeps/449) with the express intent to remove it in a future release.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix: jvm_md.h was included, but not jvm.h...

I kind of wish the __cdecl / __stdcall changes had been done separately.  Oh well.

src/hotspot/os/windows/os_windows.cpp line 5820:

> 5818: }
> 5819: 
> 5820: // FIXME

???

src/hotspot/os/windows/os_windows.cpp line 5863:

> 5861:     return nullptr;
> 5862:   }
> 5863: 

[pre-existing, and can't comment on line 5858 because it's not sufficiently near a change.]
The calculation of `len` is wasting a byte when `lib_name` is null.  The `+2` accounts for the
terminating `NUL` and the underscore separator between the sym_name part and the lib_name
part.  That underscore isn't added when there isn't a lib_name part.  I think the simplest fix would
be to change `name_len` to `(name_len +1)` and `+2` to `+1` in that calculation.  And add some
commentary.

This might be deemed not worth fixing as there is likely often no actual wastage, due to alignment
padding, and it slightly further complicates the calculation.  But additional commentary would still
be desirable, to guide the next careful reader.  In which case it might be simpler to describe the
fixed version.

Since this is pre-existing and relatively harmless in execution, it can be addressed in a followup
change.

src/hotspot/share/include/jvm.h line 1165:

> 1163: #define AGENT_ONLOAD_SYMBOLS    {"Agent_OnLoad"}
> 1164: #define AGENT_ONUNLOAD_SYMBOLS  {"Agent_OnUnload"}
> 1165: #define AGENT_ONATTACH_SYMBOLS  {"Agent_OnAttach"}

There is more cleanup that can be done here.  These definitions are used as
array initializers (hence the surrounding curly braces).  They are now always
singleton, rather than sometimes having 2 elements.  The uses iterate over the
now always singleton arrays.  Those iterations are no longer needed and could
be eliminated.  And these macros could be eliminated, using the corresponding
string directly in each use.  This can all be done as a followup change.

src/java.base/share/native/libjava/NativeLibraries.c line 67:

> 65:         strcat(jniEntryName, "_");
> 66:         strcat(jniEntryName, cname);
> 67:     }

I would prefer this be directly inlined at the sole call (in findJniFunction),
to make it easier to verify there aren't any buffer overrun problems. (I don't
think there are, but looking at this in isolation triggered warnings in my
head.)

Also, it looks like all callers of findJniFunction ensure the cname argument
is non-null. So there should be no need to handle the null case in
findJniFunction (other than perhaps an assert or something). That could be
addressed in a followup. (I've already implicitly suggested elsewhere in this
PR revising this function in a followup because of the JNI_ON[UN]LOAD_SYMBOLS
thing.)

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21744#pullrequestreview-2415002837
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1829659373
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1828969105
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1829478432
PR Review Comment: https://git.openjdk.org/jdk/pull/21744#discussion_r1829684759