RFR: 8261710: SA DSO objects have sizes that are too large

[Yasumasa Suenaga]

On Mon, 15 Feb 2021 06:43:30 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> This PR relates to [JDK-8261702](https://bugs.openjdk.java.net/browse/JDK-8261702) ( #2562 )
>> When SA creates a DSO object, which is used to represent a shared object file (.so), it initializes the "size" to be the size of the shared object file. This usually results in the size being too big. This can cause SA to get confused about whether or not an address is in the shared object. SA should instead set the DSO's size to the amount of the file that is actually mapped for executable code.
>
> These changes look good, but it would be nice to fix OSX too. Fortunately it should be easier. As part of the fix for [JDK-8247515](https://bugs.openjdk.java.net/browse/JDK-8247515), I added a `lib_info->memsz` field. I think it is correct and is what you need. I needed it in order to properly scan .dylibs for symbols without scanning too far. It seems to be working. Unfortunately we don't really have a good tests for this, but you could look at the before and after for ClhsdbPmap test to get an idea of if it is working. If you don't have an OSX machine to try, I can try out your changes for you.
> 
> BTW, I have no idea if Windows is getting the size right, but I guess we'll just have to assume it is:
> 
>     env->CallVoidMethod(obj, addLoadObject_ID, strName, (jlong) params[u].Size,
>                         (jlong) params[u].Base);

@plummercj Thanks for the review!

> These changes look good, but it would be nice to fix OSX too. Fortunately it should be easier. As part of the fix for [JDK-8247515](https://bugs.openjdk.java.net/browse/JDK-8247515), I added a `lib_info->memsz` field. I think it is correct and is what you need.

AFAICS `lib_info->memsz` is not set to loaded size, it seems to relates to the offset of the symbol in the binary.

https://github.com/openjdk/jdk/blob/34ae7aeb64db90dcb4d2f3d4acb16c714a32824f/src/jdk.hotspot.agent/macosx/native/libsaproc/libproc_impl.c#L261
https://github.com/openjdk/jdk/blob/34ae7aeb64db90dcb4d2f3d4acb16c714a32824f/src/jdk.hotspot.agent/macosx/native/libsaproc/symtab.c#L185

Can I believe `lib_info->memsz` for this purpose?
I'm not familiar of Mach-O, and I don't have Mac, so I can't evaluate it.

> I needed it in order to properly scan .dylibs for symbols without scanning too far. It seems to be working. Unfortunately we don't really have a good tests for this, but you could look at the before and after for ClhsdbPmap test to get an idea of if it is working. If you don't have an OSX machine to try, I can try out your changes for you.

In process attach on Linux, we can test the change with /proc/<PID>/maps, but in other case, we might not test it.
In coredump on Linux, it is difficult because we cannot get memory map from other source (excepts the core).

> BTW, I have no idea if Windows is getting the size right, but I guess we'll just have to assume it is:
> 
> ```
>     env->CallVoidMethod(obj, addLoadObject_ID, strName, (jlong) params[u].Size,
>                         (jlong) params[u].Base);
> ```

According to [Microsoft Docs](https://docs.microsoft.com/ja-jp/windows-hardware/drivers/ddi/dbgeng/ns-dbgeng-_debug_module_parameters), it sounds good.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2563