RFR: 8328630: Add logging when needed symbols in dll are missing. [v3]

[Robbin Ehn]

On Tue, 2 Apr 2024 11:55:30 GMT, David Holmes <dholmes at openjdk.org> wrote:

>>> `GetProcAddress` doesn't clear last-error on success, so you should only be calling `GetLastError` if the return value was null.
>> 
>> It seem like you are correct. I didn't find anything on GetProcAddress manpage, but on GetLastError page it says: "If the function is not documented to set the last-error code, the value returned by this function is simply the most recent last-error code"
>> 
>> I'll update.
>> 
>>> That said, why are you duplicating the logic from `os::lasterror` instead of just passing it the buf to fill in?
>>> 
>>> ```
>>> if (ret == nullptr) {
>>>   char buf[512];
>>>   os::lasterror(buf, sizeof(buf));
>>>   log_debug(os)(...);
>>> }
>>> ```
>> 
>> It's not clear to me that GetProcAddress will not return NULL without setting GetLastError.
>> Thus calling os::lasterror could accidentally return error string for errno instead.
>> Maybe this worry have no basis, what you think?
>
> It should be easy to test what happens when you ask for a symbol known to not exist.

Yes, but unfortunately I have no win32 env. nor access to anything other than GA for windows.
But I was thinking of the case where a Symbol have NULL as it's address.

>From some old MSDN article:
"The only required array is the Export Address Table (EAT), which is an array of function pointers that contain the address of an exported function."

AFAICT GetProcAddress just returns this "pointer" and you may have NULL pointers in this array.

Hence I think the same issue with symbol exists with NULL address as on posix but not documented.
Not all return values of NULL is actually an error.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18404#discussion_r1547761087