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

Erik Helin erik.helin at oracle.com
Fri Sep 15 09:24:46 UTC 2017


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 :)


More information about the hotspot-dev mailing list