RFR: 8187570: Comparison between pointer and char in MethodMatcher::canonicalize

[Yasumasa Suenaga]

Sorry, I forgot to paste links:

[1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-July/027431.html
email from Kim: http://mail.openjdk.java.net/pipermail/hotspot-dev/2017-July/027433.html

On 2017/09/15 21:31, Yasumasa Suenaga wrote:
> Hi Erik,
> 
> 
>> Anyways, I think Yasumasa also spotted this issue a while back, but I couldn't find a patch for it
> 
> I've pasted a patch in [1]. But I did not create webrev because Kim told me this issue was reported in JDK-8181503.
> Currently, JDK-8181503 seems to be in progress.
> 
>> Yasumasa, you want to be author and/or contributor of this patch?
> 
> Please push this patch as your changeset :-)
>>
>> Thanks,
>> Erik
>>
>> PS. First webrev and patch created with a consolidated forest, seems to be working fine :)
> 
> On 2017/09/15 18:24, Erik Helin wrote:
>> Hi all,
>>
>> when I compiled with gcc 7.1.1 it warned me about the following code:
>>
>> bool MethodMatcher::canonicalize(char * line, const char *& error_msg) {
>>    char* colon = strstr(line, "::");
>>    bool have_colon = (colon != NULL);
>>    if (have_colon) {
>>      // Don't allow multiple '::'
>>      if (colon + 2 != '\0') {
>>
>> The problem is that colon is a pointer, so colon + 2 is a pointer, and then colon + 2 is compared '\0', which is a char :/
>>
>> Anyways, I think Yasumasa also spotted this issue a while back, but I couldn't find a patch for it, so I quickly whipped one up:
>>
>> http://cr.openjdk.java.net/~ehelin/8187570/00/
>>
>> --- old/src/hotspot/share/compiler/methodMatcher.cpp    2017-09-15 10:43:49.430656504 +0200
>> +++ new/src/hotspot/share/compiler/methodMatcher.cpp    2017-09-15 10:43:49.102654877 +0200
>> @@ -96,7 +96,7 @@
>>     bool have_colon = (colon != NULL);
>>     if (have_colon) {
>>       // Don't allow multiple '::'
>> -    if (colon + 2 != '\0') {
>> +    if (colon[2] != '\0') {
>>         if (strstr(colon+2, "::")) {
>>           error_msg = "Method pattern only allows one '::' allowed";
>>           return false;
>>
>> I was a little bit afraid of what would happen if line (the parameter to MethodMatcher::canonicalize) isn't properly null terminated, so I checked the callers, and it seems like all callers property null terminate the `line` argument.
>>
>> Yasumasa, you want to be author and/or contributor of this patch?
>>
>> Thanks,
>> Erik
>>
>> PS. First webrev and patch created with a consolidated forest, seems to be working fine :)