RFR: 8274329: Fix non-portable HotSpot code in MethodMatcher::parse_method_pattern [v2]

Ioi Lam iklam at openjdk.java.net
Wed Oct 6 04:35:07 UTC 2021

On Wed, 6 Oct 2021 02:33:30 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Hi all,
>> I tried to build OpenJDK on Cygwin (Windows 2016 + VS2019).
>> However, I failed with C4474 and C4778 warnings as below:
>> Compiling 100 properties into resource bundles for java.desktop
>> Compiling 3038 files for java.base
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): error C2220: the following warning is treated as an error
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning C4778: 'sscanf' : unterminated format string '%255[*\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e\x97%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): warning C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(269): note: placeholders and their parameters expect 1 variadic arguments, but 3 were provided
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning C4778: 'sscanf' : unterminated format string '%1022[[);/\x01\x02\x03\x04\x05\x06\a\b\n\v\f\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f!"#$%&'*+,-0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ\^_`abcdefghijklmnopqrstuvwxyz{|}~\xe2\x82\xac\xe4\xba\x97\xe5\x84\x8e\xe5\x8e%n'
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): warning C4474: 'sscanf' : too many arguments passed for format string
>> e:\jiefu\ws\jdk\src\hotspot\share\compiler\methodMatcher.cpp(319): note: placeholders and their parameters expect 0 variadic arguments, but 2 were provided
>> The failure is caused by non-ASCII chars in the format string of sscanf [1][2], which is non-portable on our Windows platform.
>> In fact, these non-ASCII coding also triggers C4819 warning, which had been disabled in JDK-8216154 [3].
>> And I also found an article showing that sscanf may fail with non-ASCII in the format string [4].
>> So it would be nice to remove these non-ASCII chars  (`\x80 ~ \xef`).
>> And I think it's safe to do so.
>> This is because:
>>  1) There are actually no non-ASCII chars for package/class/method/signature names.
>>  2) I don't think there is a use case, in which people will input non-ASCII for `CompileCommand`.
>> You may argue that the non-ASCII may be used by the parser itself.
>> But I didn't find that usage at all. (Please let me know if I miss something.)
>> So I suggest to remove these non-ASCII code to make HotSpot to be more portable.
>> And if we do so, we can also remove the only one `PRAGMA_DISABLE_MSVC_WARNING(4819)` [5].
>> Testing:
>>  - Build tests on Windows
>>  - tier1~3 on Linux/x64
>> Thanks.
>> Best regards,
>> Jie
>> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L269
>> [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L319
>> [3] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-January/032014.html
>> [4] https://jeffpar.github.io/kbarchive/kb/047/Q47369/
>> [5] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/compiler/methodMatcher.cpp#L246
> Jie Fu has updated the pull request incrementally with one additional commit since the last revision:
>   Disable non-ASCII for Windows only

The idea looks good to me. I just have a suggestion to make the code more readable.

src/hotspot/share/compiler/methodMatcher.cpp line 77:

> 75:     "\x60\x61\x62\x63\x64\x65\x66\x67\x68\x69\x6a\x6b\x6c\x6d\x6e\x6f" \
> 76:     "\x70\x71\x72\x73\x74\x75\x76\x77\x78\x79\x7a\x7b\x7c\x7d\x7e\x7f"
> 77: #endif

It's hard to tell what's the difference between these two RANGEBASE definitions. How about doing it like this to make the code more readable?

#define RANGEBASE_ASCII "....."
#define RANGEBASE_NON_ASCII "....."
#ifdef WINDOWS


Marked as reviewed by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5704

More information about the hotspot-runtime-dev mailing list