Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
On 07/03/2013 11:54, Staffan Larsen wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan This looks good to me too. A minor nit is that probably should put a space in while(path = ... in all of the dll_build_name implementation. Ignore this comment if you've already created the change-set are ready to push of course. -Alan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Can I get an official Review of this change? Thanks, /Staffan On 7 mar 2013, at 12:54, Staffan Larsen staffan.lar...@oracle.com wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; == Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
I already reviewed this, and it is a good fix. Please, see my email attached. Thanks, Serguei On 3/18/13 7:13 AM, Staffan Larsen wrote: Can I get an official Review of this change? Thanks, /Staffan On 7 mar 2013, at 12:54, Staffan Larsen staffan.lar...@oracle.com mailto:staffan.lar...@oracle.com wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug:http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan ---BeginMessage--- My vpn was disconnected when I sent this. Resending to make sure you've got it. Thanks, Serguei On 3/7/13 9:39 AM, serguei.spit...@oracle.com wrote: Nice fix! This is more clear and robust. Extra memory allocation is Ok. Thanks, Serguei On 3/7/13 3:54 AM, Staffan Larsen wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks,
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Hi Serguei, On 19/03/2013 2:47 AM, serguei.spit...@oracle.com wrote: I already reviewed this, and it is a good fix. Please, see my email attached. You are not an OpenJDK Reviewer [1] . Staffan needs an official Review from a Reviewer before it can be pushed. That doesn't mean your review is not wanted or appreciated - it mostly certain is - but an official Review is still required. Cheers, David [1] http://openjdk.java.net/bylaws#reviewer Thanks, Serguei On 3/18/13 7:13 AM, Staffan Larsen wrote: Can I get an official Review of this change? Thanks, /Staffan On 7 mar 2013, at 12:54, Staffan Larsen staffan.lar...@oracle.com mailto:staffan.lar...@oracle.com wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug:http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; == Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Nice fix! This is more clear and robust. Extra memory allocation is Ok. Thanks, Serguei On 3/7/13 3:54 AM, Staffan Larsen wrote: Here is a webrev for fixing this problem. I actually removed all of our own tokenization code in dll_build_name() and replaced it with calls to strtok_r (strtok_s on windows) instead. I think this should be more robust, at the cost of an extra memory allocation. I've also added the const qualifier to some of the parameters. http://cr.openjdk.java.net/~sla/8009558/webrev.02/ http://cr.openjdk.java.net/%7Esla/8009558/webrev.02/ All of the jdi and hprof tests passes with this change. Thanks, /Staffan On 6 mar 2013, at 20:48, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug:http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; == Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Staffan, Thank you for the confirmation and taking care about this issue! Thanks, Serguei On 3/6/13 11:36 AM, Staffan Larsen wrote: Nice catch, Serguei! Unfortunately I have already pushed my change, but I filed JDK-8009558: linked_md.c::dll_build_name can get stuck in an infinite loop to track this new problem and I am working on a fix. Thanks, /Staffan On 5 mar 2013, at 20:26, serguei.spit...@oracle.com mailto:serguei.spit...@oracle.com wrote: Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug:http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev:http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Thanks Alan! On 5 mar 2013, at 15:19, Alan Bateman alan.bate...@oracle.com wrote: On 04/03/2013 18:56, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: : bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ This is a good find and the changes looks good to me. -Alan.
Re: RFR(S): 8009397 test/com/sun/jdi/PrivateTransportTest.sh: ERROR: transport library missing onLoad entry: private_dt_socket
Hi Staffan, Thank you for this discovery! It looks good, but I have a couple of comments. It seems, there is one more problem in this code: 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue;== Endless loop if we hit this line 71 } Do we need to do 'pathname++' before continuing at the line #70? It is going to be endless loop in cases there is a PATH_SEPARATOR at the beginning of paths or two PATH_SEPARATOR's in a row. These would be incorrect path lists but the code above is targeting exactly such cases. Also, the argument name pathname in the original code is confusing. Should we rename it to pathnames? Thanks, Serguei On 3/4/13 10:56 AM, Staffan Larsen wrote: I accidentally stepped on this bug the other day. There is a problem in linker_md.c : dll_build_name() where an internal pointer can be moved to point outside the string. The code looks like: 57 static void dll_build_name(char* buffer, size_t buflen, 58const char* pname, const char* fname) { 59 // Based on os_solaris.cpp 60 61 char *path_sep = PATH_SEPARATOR; 62 char *pathname = (char *)pname; 63 while (strlen(pathname) 0) { 64 char *p = strchr(pathname, *path_sep); 65 if (p == NULL) { 66 p = pathname + strlen(pathname); 67 } 68 /* check for NULL path */ 69 if (p == pathname) { 70 continue; 71 } 72 (void)snprintf(buffer, buflen, %.*s/lib%s. LIB_SUFFIX, (p - pathname), 73pathname, fname); 74 75 if (access(buffer, F_OK) == 0) { 76 break; 77 } 78 pathname = p + 1; 79 *buffer = '\0'; 80 } If the supplied pname is a buffer with a simple path without any path separators in it, p will be set to the terminating nul on line 66. Then on line 78 it will be moved outside the buffer. Fixing this also necessitates fixes to the callers to check for an empty return string (in buffer). The same code show up in both the solaris code and the windows code as well as hprof. bug: http://bugs.sun.com/view_bug.do?bug_id=8009397 webrev: http://cr.openjdk.java.net/~sla/8009397/webrev.00/ Thanks, /Staffan