RFR: 8241439: jdk.NativeLibraryEvent hooks all opened regular files

[Yasumasa Suenaga]

Hi Denghui,

On 2020/04/09 19:39, Denghui Dong wrote:
> Hi Yasumasa,
> 
> I have another question:
> if a module is loaded into several discontinuous segments, I think this patch can't cover this situation, am I right?

I think all executables would load into continuous memory spaces in Linux.

Linux would load ELF binary at load_elf_binary(). This function loads all loadable segments (PT_LOAD)
and start / end addresses are stored to mm_struct [1].

If the application attempt to load same library in twice with dlopen(3), manpage [2] says same handle is returned.
So the library would not deploy to discontinuous space.


Thanks,

Yasumasa


[1] https://github.com/torvalds/linux/blob/master/fs/binfmt_elf.c#L1103
[2] https://linux.die.net/man/3/dlopen


> Denghui
> 
>     ------------------------------------------------------------------
>     From:Yasumasa Suenaga <suenaga at oss.nttdata.com>
>     Send Time:2020年4月9日(星期四) 17:05
>     To:董登辉(卓昂) <denghui.ddh at alibaba-inc.com>; Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>; hotspot-jfr-dev at openjdk.java.net <hotspot-jfr-dev at openjdk.java.net>; hotspot-jfr-dev <hotspot-jfr-dev-bounces at openjdk.java.net>
>     Cc:yasuenag at gmail.com <yasuenag at gmail.com>
>     Subject:Re: RFR: 8241439: jdk.NativeLibraryEvent hooks all opened regular files
> 
>     Hi Denghui,
> 
>     Thank you for the comment.
>     I uploaded new webrev:
> 
>         http://cr.openjdk.java.net/~ysuenaga/JDK-8241439/webrev.01/
> 
>     On 2020/04/09 17:11, Denghui Dong wrote:
>      > Hi Yasumasa,
>      > I'm not a reviewer, but the following code looks strange:
>      > =====
>      > 2096       if ((base == NULL) && (base < phdr_base)) {
>      > =====
>      >
>      > if the condition is right, it could be simplified: if (base < phdr_base),
>      > but IMO, the correct condition should be if (base > phdr_base), I may be wrong. ��
> 
>     You are right, condition should be (base > phdr_base). We need to store base address to `base` from all PT_LOAD header.
>     So NULL check is needed because (base > phdr_base) always false without it.
> 
> 
>     Thanks,
> 
>     Yasumasa
> 
> 
>      > Thanks,
>      > Denghui
>      > ------------------------------------------------------------------
>      > From:Yasumasa Suenaga <suenaga at oss.nttdata.com>
>      > Send Time:2020年4月9日(星期四) 15:01
>      > To:Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>; hotspot-jfr-dev at openjdk.java.net <hotspot-jfr-dev at openjdk.java.net>
>      > Cc:yasuenag at gmail.com <yasuenag at gmail.com>
>      > Subject:RFR: 8241439: jdk.NativeLibraryEvent hooks all opened regular files
>      >
>      > Hi all,
>      >
>      > Please review this change:
>      >
>      >     JBS: https://bugs.openjdk.java.net/browse/JDK-8241439
>      >     webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8241439/webrev.00/
>      >
>      > HotSpot has os::get_loaded_modules_info() to get all loaded native libraries, however it would call callback in twice on same library on Linux. Also it would call callback even if the file is not a library.
>      >
>      > We can see this problem in jdk.NativeLibrary JFR event as below:
>      >
>      >
>      > ```
>      > jdk.NativeLibrary {
>      >      startTime = 16:59:06.621
>      >      name = "/usr/lib64/libc-2.30.so"
>      >      baseAddress = 0x7FDB06ACC000
>      >      topAddress = 0x7FDB06AF1000
>      > }
>      >
>      > jdk.NativeLibrary {
>      >      startTime = 16:59:06.621
>      >      name = "/usr/lib64/libc-2.30.so"
>      >      baseAddress = 0x7FDB06AF1000
>      >      topAddress = 0x7FDB06C40000
>      > }
>      >
>      >       :
>      >
>      > ```
>      >
>      > ```
>      > jdk.NativeLibrary {
>      >      startTime = 16:59:06.621
>      >      name = "/home/ysuenaga/OpenJDK/jdk/build/linux-x86_64-server-fastdebug/images/jdk/lib/server/classes.jsa"
>      >      baseAddress = 0x800000000
>      >      topAddress = 0x800007000
>      > }
>      > ```
>      >
>      > This change uses dl_iterate_phdr() to get them, so these problem has gone.
>      >
>      >
>      > Thanks,
>      >
>      > Yasumasa
>      > 
> 
>