RFR(M): 8140482: Various minor code improvements (runtime)

[Dmitry Samersoff]

Goetz,

Sorry for being later:

1. libproc_impl.c

it might be better to check that strlen(alt_root) + strlen(name) is less
than PATH_MAX at ll. 50 and return an error. Then we can safely leave
strcat staff as it is.


2. ps_core.c

if interpreter name is truncated for some reason, we have no chance to
open it and can't continue.

So it might be better to check if exec_php->p_filesz > BUF_SIZE and
return an error immediately.

Also, please add a comment that BUF_SIZE is (PATH_MAX + NAME_MAX + 1)


3. os_linux.cpp

4257:

I'm OK with this change but we should revisit the use of MAXSIGNUM under
Linux and probably define it to _NSIG for the rest of VM

5944:

What is the reason of the change? core_pattern is initialized to zero at
ll. 5939.

It might be better to check that ret is less than corep_pattern_len and
then use ret instead of strlen at ll. 5948


deoptimization.cpp:

2085, 2182:

jio_snprintf always return -1 and always null-terminate string in case
of overflow.  So original code is incorrect and we can just remove it.


attachListener.hpp:

125,143:

It might be better to calculate strlen(name) once, than use memcpy.

heapDumper.cpp:

1983:

   we checked that HeapDumpPath fits in the buffer at ll 1973, so we can
safely use strcpy() here.

xmlstream.cpp:

354:

   What is the reason of this change? we checked that string fits to
buffer at ll 348.


-Dmitry





On 2015-10-27 10:39, Lindenmaier, Goetz wrote:
> Hi,
> 
> SAP requires us to fix a row of issues in hotspot.  I'd like to share these
> with openjdk:
> http://cr.openjdk.java.net/~goetz/webrevs/8140482-covRt/webrev.00
> 
> Please review this change.  I please need a sponsor.
> 
> The fixes in detail:
> 
> libproc_impl.c:
> Do strncpy in case getenv returned a bad string.
> Strcat could overflow the buffer. Use strncat instead.
> 
> ps_core.c:
> Pread not necessarily terminates interp_name which is printed thereafter.
> Increase buffer size by 1 and add '\0'.
> 
> stubRoutines_x86.cpp:
> Cast to proper type. This way, left and right of '&' have the same type.
> 
> attachListener_linux.cpp:
> Read does not terminate buf. Size for '\0' is already considered.
> 
> os_linux.cpp:
> Array sigflags[] has size MAXSIGNUM==32.  _NSIG is bigger than
> MAXSIGNUM (_NSIG == 65 on my machine).
> sig is checked to be smaller than _NSIG. Later, in set_our_sigflags(),
> sig is used to access sigflags[MAXSIGNUM] which can overflow the array.
> Should we also increase MAXSIGNUM?
> os::get_core_path(): read does not terminate string, but strlen is
> called on it.  The size already foresees one char for the '\0' byte.
> 
> codeBuffer.cpp:
> New_capacity is not initialized. Figure_expanded_capacities() handles this
> correctly, but initializing this is cheap and safe.
> 
> dict.cpp:
> If j-- is executed for j==0, the loop aborts because j is unsigned (0-- >= b->_cnt).
> Instead, only do j++ if necessary.
> 
> generateOopMap.cpp:
> Idx is read from String. This is only called with constant strings, so compare
> should be folded away by optimizing compilers if inlined.
> 
> deoptimization.cpp:
> If buflen == 0, buf[-1] is accessed.
> 
> task.cpp:
> Fatal can return if -XX:SuppressErrorAt is used. Just don't access the
> array in this case.
> 
> attachListener.hpp:
> Do strncpy to not overflow buffer. Don't write more chars than before.
> 
> heapDumper.cpp:
> strncpy does not null terminate.
> 
> 
> Some of these, as the issue in codeBuffer.cpp, are actually handled correctly.
> Nevertheless this is not that obvious so that somebody changing the code
> Could oversee he has to add the initialization.
> 
> Some of these fixes are part of SAP JVM for a long time.  This change has
> been tested with our nightly build of openJDK.
> 
> Best regards,
>   Goetz,.
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.