XS RFR: 8006001: [parfait] Possible file leak in hotspot/src/os/linux/vm/perfMemory_linux.cpp

[David Holmes]

This may be too late but ...

On 4/04/2013 3:42 AM, Calvin Cheung wrote:
> Fixing the following 2 warnings reported by the parfait tool:
>
> 1) fd leaks when ThreadShadow::has_pending_exception(((unresolved
> type*)__the_thread__)) is true at line 897
>      (I couldn't reproduce this warning with parfait even with the
> "--warning" option)
>
> Fix: instead of using THROW_MSG_0 which returns 0, use THROW_MSG_ with
> return value of OS_ERR.
>         There's already an assert(fd != OS_ERR) after calling
> open_sharedmem_file().

I don't understand the problem or the fix. Whether we use THROW_MSG_0 or 
THROW_MSG_(..., OS_ERR) line 897:

897   fd = open_sharedmem_file(rfilename, file_flags, CHECK);

is simply going to trigger a return due to the CHECK macro. And if we 
posted an exception it means the ::open failed and so we can not 
possibly be leaking a fd.

???

> 2) Possible uninitialised variable (CWE 457): Uninitialised variable
> 'size' may be used as argument 2 when calling mmap at line 905 of
> src/os/linux/vm/perfMemory_linux.cpp
>
> Fix: add an assert(size > 0) before calling ::mmap().

Again I don't understand the fix. It seems to me this current code is 
broken and that really we should have:

  900   if (*sizep == 0) {
  901     size = sharedmem_filesize(fd, CHECK);
  902   }
        else {
          size = *sizep;
        }

The initialization to zero might fix parfait but the code still seems 
wrong to me.

David
-----

> webrev: http://http://cr.openjdk.java.net/~ccheung/8006001/
> <http://cr.openjdk.java.net/%7Eccheung/8006001/>
> bug: https://jbs.oracle.com/bugs/browse/JDK-8006001
>
> Tests:
>      JPRT
>      parfait
>
> thanks,
> Calvin