RFR: 8364816: GetLastError() in os_windows.cpp should not store value to errno [v7]

[Joel Sikström]

On Mon, 1 Sep 2025 09:51:30 GMT, Anton Artemov <duke at openjdk.org> wrote:

>> Hi, please consider the following changes:
>> 
>> In `os_windows.cpp` in a few places results returned by `GetLastError()` are stored to `errno`. However, `errno` has no relation to `GetLastError()`, and their ranges are not the same.
>> 
>> Results of `GetLastError()` should be stored into variables of type `DWORD`.
>> 
>> The removed section in `src/hotspot/share/cds/aotClassLocation.cpp` was relying on values returned by `GetLastError()` and stored to `errno` in `os::stat()`. Though the logic was correct, these values should not be stored to `errno`. The functionality is preserved by storing a **valid** value `ENOENT` to `errno` in `os::stat()`. 
>> 
>> Tested in tiers 1 - 3.
>
> Anton Artemov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/hotspot/os/windows/os_windows.cpp
>   
>   8364816: Added _set_errno(0) to avoid reading old values.
>   
>   Co-authored-by: Joel Sikström <joel.sikstrom at oracle.com>

Thank you for this, I think this is a step in the right direction.

I think this looks good now, since there is no change in external behavior of using `os::stat()`. I agree that setting errno to `GetLastError()` was broken, which is now fixed, nice! But reading errno after `os::stat()` is as broken before as it is now and should really be fixed in a follow-up. Users of `os::stat()` should only check if the call is successful or not, until we have a robust method in place for detecting specific errors.

It would have been nice to fix `os::stat()` before this, which would have meant we didn't have to consider the specific use-case in `aotClassLocation.cpp` which reads errno.

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

Marked as reviewed by jsikstro (Committer).

PR Review: https://git.openjdk.org/jdk/pull/26901#pullrequestreview-3172971080