RFR(M) 8193373: Cleanup ElfFile and family

yumin qi yumin.qi at gmail.com
Thu Feb 8 17:51:53 UTC 2018


Hi, Zhengyu

  Looks good to me. One minor comment:

+bool ElfFile::specifies_noexecstack(const char* filepath) {+  //
Returns true if the elf file is marked NOT to require an executable
stack,+  // or if the file could not be opened.+  // Returns false if
the elf file requires an executable stack, the stack flag+  // is not
set at all, or if the file can not be read.



  should the comments in side the function be moved above the function?

   others looks OK to me.

Thanks
Yumin






On Mon, Feb 5, 2018 at 8:08 AM, Zhengyu Gu <zgu at redhat.com> wrote:

> Please review this cleanup and refactoring of elf decoder.
>
> * Fixed coding styles.
>   - Moved non-static members to the top of classes
>   - Fixed variable names (from Windows naming style to hotspot style)
>
> * Moved Linux implementation of ElfFile::specifies_noexecstack to os
>   directory
>
> * Refactored section cache logic into common ElfSection class.
>
> * Added utility classes FileReader and MarkedFileReader to cleanup
>   duplicated code.
>
> * Deleted section header string table once initialization is completed,
>   cause it is not used for decoding.
>
> * When decoder failed to cache section data, current implementation
>   tries to fallback to read data from file directly. This never
>   worked, cause it put decoder into out of memory state when it failed
>   load section data, so decoder always bailed out prematurely.
>   This patch also fixed this bug.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8193373
> Webrev: http://cr.openjdk.java.net/~zgu/8193373/webrev.00/
>
>
> Test:
>
>   hotspot_runtime on Linux 64 (fastdebug and release)
>
> Thanks,
>
> -Zhengyu
>


More information about the hotspot-runtime-dev mailing list