On Mon, 15 Feb 2021 06:43:30 GMT, Chris Plummer <[email protected]> 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
