Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 03:19:51 GMT, Ioi Lam wrote: > > Given that this seems to be a common problem in our code, and likely a very > > very old problem at that, why has it never been reported before? I'm not > > questioning the fix except to the extent that I'm questioning our > > understanding of the problem. > > I think it's probably because the errors only happen in very rare cases. For > example, when a DLL is missing from the JAVA_HOME. For more common cases, > such as Process_impl.c, the code has been rewritten to use FormatMessageW, > which is unicode. In addition, this problem can be seen on non-ASCII locales such as CJK, so I guess many people overlooked it. And also I think it is not just a problem on Windows because some older Linux machines on Japan set locale to EUC JP. In case of ProcessImpl_md.c which @iklam pointed, result of `FormatMessageW()` which is encoded WideString (UTF-16) will be converted to UTF-8 with `WideCharToMultiByte()` at `win32Error()`, so it should work fine. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 03:07:49 GMT, Chris Plummer wrote: > Given that this seems to be a common problem in our code, and likely a very > very old problem at that, why has it never been reported before? I'm not > questioning the fix except to the extent that I'm questioning our > understanding of the problem. I think it's probably because the errors only happen in very rare cases. For example, when a DLL is missing from the JAVA_HOME. For more common cases, such as Process_impl.c, the code has been rewritten to use FormatMessageW, which is unicode. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 02:40:29 GMT, Yasumasa Suenaga wrote: >> Marked as reviewed by iklam (Reviewer). > > @iklam Thanks for your review! > >> I looked at a cases in the JDK code where >> `Java_sun_security_pkcs11_wrapper_PKCS11_connect()` calls `FormatMessage()`. >> It eventually passes the `char*` to `jni_ThrowNew`, which eventually gets to >> `Exceptions::new_exception` with `to_utf8_safe==safe_to_utf8`. This means >> the message will be converted with `java_lang_String::create_from_str()`, >> which does NOT call `JNU_NewStringPlatform`. So I think in this case, the >> error message will also be garbled. >> >> java.base/windows/native/libnet/Inet4AddressImpl.c seems to have a similar >> problem in the `ping4` function. > > Agree. `ping4()` calls `NET_ThrowNew()` which passes `ThrowNew()` JNI > function without modifying arguments. > I looked at other cases in JDK code, following places has possible to garble: > > * `throwByName()` @ jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c > * `Java_sun_security_pkcs11_Secmod_nssLoadLibrary()` @ j2secmod_md.c > * jdk.jdi/share/native/libdt_shmem/SharedMemoryTransport.c > * `throwShmemException()` @ SharedMemoryTransport.c > >> But, I don't know how to write a simple test case for the above. > > Agree, we might need to fix garbled messages one by one when we find out > issues. > IMHO we need to establish rule that we have to use `JNU_NewStringPlatform()` > if we want to throw exception with message from platform (`FormatMessage()`, > `strerror()`) from JNI. Given that this seems to be a common problem in our code, and likely a very very old problem at that, why has it never been reported before? I'm not questioning the fix except to the extent that I'm questioning our understanding of the problem. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 02:19:37 GMT, Ioi Lam wrote: >> I got garbled exception message as following when I run `livenmethods` >> CLHSDB command: >> >> sun.jvm.hotspot.debugger.DebuggerException : ?w???W >> >> My Windows laptop is set Japanese Locale, garbled message was written in >> Japanese. >> saproc.dll would throw exception via >> [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) >> JNI function, but it accepts UTF-8 encoded message. However >> [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) >> Windows API might not return UTF-8 encoded string on Japanese locale. >> >> java.dll (libjava,so) provides good functions to resolve this issue. We can >> convert localized (non ascii) chars to UTF-8 string. I use them in this PR >> and remove `FormatMessage()` call from sadis.c. >> And also I remove `-D_MBCS` from compiler option because [MBCS has been >> already >> deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) >> - it does not seem to add to any other executables. > > Marked as reviewed by iklam (Reviewer). @iklam Thanks for your review! > I looked at a cases in the JDK code where > `Java_sun_security_pkcs11_wrapper_PKCS11_connect()` calls `FormatMessage()`. > It eventually passes the `char*` to `jni_ThrowNew`, which eventually gets to > `Exceptions::new_exception` with `to_utf8_safe==safe_to_utf8`. This means the > message will be converted with `java_lang_String::create_from_str()`, which > does NOT call `JNU_NewStringPlatform`. So I think in this case, the error > message will also be garbled. > > java.base/windows/native/libnet/Inet4AddressImpl.c seems to have a similar > problem in the `ping4` function. Agree. `ping4()` calls `NET_ThrowNew()` which passes `ThrowNew()` JNI function without modifying arguments. I looked at other cases in JDK code, following places has possible to garble: * `throwByName()` @ jdk.crypto.cryptoki/share/native/libj2pkcs11/p11_util.c * `Java_sun_security_pkcs11_Secmod_nssLoadLibrary()` @ j2secmod_md.c * jdk.jdi/share/native/libdt_shmem/SharedMemoryTransport.c * `throwShmemException()` @ SharedMemoryTransport.c > But, I don't know how to write a simple test case for the above. Agree, we might need to fix garbled messages one by one when we find out issues. IMHO we need to establish rule that we have to use `JNU_NewStringPlatform()` if we want to throw exception with message from platform (`FormatMessage()`, `strerror()`) from JNI. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 01:31:28 GMT, Chris Plummer wrote: > > jdk.hotspot.agent do not have `FormatMessage()` call in other place. > > Did you say about whole JDK code? I haven't checked all of them, but some > > code (e.g. net_util_md.c) uses `JNU_ThrowByName()` which is provided by > > java.dll. > > Yes, I was referring to all JDK code. Given how many references there are to > FormatMessage(), I'm surprised this issue has not turned up before. I was > wondering if there is something unique about the SA reference that makes the > issue only turn up there. I looked at a cases in the JDK code where `Java_sun_security_pkcs11_wrapper_PKCS11_connect()` calls `FormatMessage()`. It eventually passes the `char*` to `jni_ThrowNew`, which eventually gets to `Exceptions::new_exception` with `to_utf8_safe==safe_to_utf8`. This means the message will be converted with `java_lang_String::create_from_str()`, which does NOT call `JNU_NewStringPlatform`. So I think in this case, the error message will also be garbled. java.base/windows/native/libnet/Inet4AddressImpl.c seems to have a similar problem in the `ping4` function. But, I don't know how to write a simple test case for the above. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. Marked as reviewed by iklam (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 01:30:07 GMT, Ioi Lam wrote: > Now the saproc DLL has an external reference to getLastErrorString, > JNU_NewStringPlatform and JNU_NewObjectByName on all platforms. Do we need to > modify the makefiles for platforms other than Windows? @iklam Agree, so I added `LIBS_unix := -ljava` to make/modules/jdk.hotspot.agent/Lib.gmk in this change. It works fine on Windows x64 and Linux x64. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 00:41:11 GMT, Yasumasa Suenaga wrote: >> There are probably 25 or so places in our code where we use FormatMessage to >> get the error message. Are these all going to run into the same >> FormateMessage bug? >> >> Also, it's not clear to me why you are getting garbled text in the first >> place. You said "Windows API might not return UTF-8 encoded string on >> Japanese locale." Why is that the case? > >> There are probably 25 or so places in our code where we use FormatMessage to >> get the error message. Are these all going to run into the same >> FormateMessage bug? > > jdk.hotspot.agent do not have `FormatMessage()` call in other place. > Did you say about whole JDK code? I haven't checked all of them, but some > code (e.g. net_util_md.c) uses `JNU_ThrowByName()` which is provided by > java.dll. > >> Also, it's not clear to me why you are getting garbled text in the first >> place. You said "Windows API might not return UTF-8 encoded string on >> Japanese locale." Why is that the case? > > Japanese locale on Windows uses CP932., so `FormatMessage()` would return > error message with Japanese chars which are encoded by CP932. It is not UTF-8. Now the saproc DLL has an external reference to getLastErrorString, JNU_NewStringPlatform and JNU_NewObjectByName on all platforms. Do we need to modify the makefiles for platforms other than Windows? - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Tue, 5 Jan 2021 00:41:11 GMT, Yasumasa Suenaga wrote: > jdk.hotspot.agent do not have `FormatMessage()` call in other place. > Did you say about whole JDK code? I haven't checked all of them, but some > code (e.g. net_util_md.c) uses `JNU_ThrowByName()` which is provided by > java.dll. Yes, I was referring to all JDK code. Given how many references there are to FormatMessage(), I'm surprised this issue has not turned up before. I was wondering if there is something unique about the SA reference that makes the issue only turn up there. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 21:06:46 GMT, Chris Plummer wrote: > There are probably 25 or so places in our code where we use FormatMessage to > get the error message. Are these all going to run into the same > FormateMessage bug? jdk.hotspot.agent do not have `FormatMessage()` call in other place. Did you say about whole JDK code? I haven't checked all of them, but some code (e.g. net_util_md.c) uses `JNU_ThrowByName()` which is provided by java.dll. > Also, it's not clear to me why you are getting garbled text in the first > place. You said "Windows API might not return UTF-8 encoded string on > Japanese locale." Why is that the case? Japanese locale on Windows uses CP932., so `FormatMessage()` would return error message with Japanese chars which are encoded by CP932. It is not UTF-8. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 20:58:59 GMT, Chris Plummer wrote: >> I got garbled exception message as following when I run `livenmethods` >> CLHSDB command: >> >> sun.jvm.hotspot.debugger.DebuggerException : ?w???W >> >> My Windows laptop is set Japanese Locale, garbled message was written in >> Japanese. >> saproc.dll would throw exception via >> [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) >> JNI function, but it accepts UTF-8 encoded message. However >> [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) >> Windows API might not return UTF-8 encoded string on Japanese locale. >> >> java.dll (libjava,so) provides good functions to resolve this issue. We can >> convert localized (non ascii) chars to UTF-8 string. I use them in this PR >> and remove `FormatMessage()` call from sadis.c. >> And also I remove `-D_MBCS` from compiler option because [MBCS has been >> already >> deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) >> - it does not seem to add to any other executables. > > src/jdk.hotspot.agent/share/native/libsaproc/sadis.c line 75: > >> 73: >> 74: #ifdef _WINDOWS >> 75: static int getLastErrorString(char *buf, size_t len) > > Is this being removed because a copy already exists in jni_util_md.c, and you > now have access to this copy because you are linking against java.dll? Exactly. - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. There are probably 25 or so places in our code where we use FormatMessage to get the error message. Are these all going to run into the same FormateMessage bug? Also, it's not clear to me why you are getting garbled text in the first place. You said "Windows API might not return UTF-8 encoded string on Japanese locale." Why is that the case? src/jdk.hotspot.agent/share/native/libsaproc/sadis.c line 75: > 73: > 74: #ifdef _WINDOWS > 75: static int getLastErrorString(char *buf, size_t len) Is this being removed because a copy already exists in jni_util_md.c, and you now have access to this copy because you are linking against java.dll? - PR: https://git.openjdk.java.net/jdk/pull/1928
Re: RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
On Mon, 4 Jan 2021 09:25:55 GMT, Yasumasa Suenaga wrote: > I got garbled exception message as following when I run `livenmethods` CLHSDB > command: > > sun.jvm.hotspot.debugger.DebuggerException : ?w???W > > My Windows laptop is set Japanese Locale, garbled message was written in > Japanese. > saproc.dll would throw exception via > [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) > JNI function, but it accepts UTF-8 encoded message. However > [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) > Windows API might not return UTF-8 encoded string on Japanese locale. > > java.dll (libjava,so) provides good functions to resolve this issue. We can > convert localized (non ascii) chars to UTF-8 string. I use them in this PR > and remove `FormatMessage()` call from sadis.c. > And also I remove `-D_MBCS` from compiler option because [MBCS has been > already > deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) > - it does not seem to add to any other executables. Build changes look ok to me, but someone from serviceability needs to comment on the actual change. - Marked as reviewed by erikj (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1928
RFR: 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale
I got garbled exception message as following when I run `livenmethods` CLHSDB command: sun.jvm.hotspot.debugger.DebuggerException : ?w???W My Windows laptop is set Japanese Locale, garbled message was written in Japanese. saproc.dll would throw exception via [ThrowNew()](https://docs.oracle.com/en/java/javase/15/docs/specs/jni/functions.html#thrownew) JNI function, but it accepts UTF-8 encoded message. However [FormatMessage()](https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage) Windows API might not return UTF-8 encoded string on Japanese locale. java.dll (libjava,so) provides good functions to resolve this issue. We can convert localized (non ascii) chars to UTF-8 string. I use them in this PR and remove `FormatMessage()` call from sadis.c. And also I remove `-D_MBCS` from compiler option because [MBCS has been already deprecated](https://docs.microsoft.com/ja-jp/cpp/text/support-for-multibyte-character-sets-mbcss) - it does not seem to add to any other executables. - Commit messages: - 8259045: Exception message from saproc.dll is garbled on Windows with Japanese locale Changes: https://git.openjdk.java.net/jdk/pull/1928/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1928=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259045 Stats: 50 lines in 2 files changed: 4 ins; 37 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/1928.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1928/head:pull/1928 PR: https://git.openjdk.java.net/jdk/pull/1928
