On Wed, 17 Feb 2021 06:28:09 GMT, Yasumasa Suenaga <[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. > > Yasumasa Suenaga has updated the pull request incrementally with one > additional commit since the last revision: > > Use p_filesz instead of p_memsz Hi Yasumasa, Just some comments. src/jdk.hotspot.agent/linux/native/libsaproc/libproc_impl.c +static inline uintptr_t align_down(uintptr_t ptr, size_t size) { + return (ptr & ~(size - 1)); +} + +static inline uintptr_t align_up(uintptr_t ptr, size_t size) { + return ((ptr + size - 1) & ~(size - 1)); +} The name of 'size' parameter is confusing. Should it be renamed to something like page_size of psize? + lib->end = (uintptr_t)-1L; . . . + if ((lib->end == (uintptr_t)-1L) || (lib->end < aligned_end)) { + lib->end = aligned_end; } The condition `(lib->end == (uintptr_t)-1L)` is a subset of `(lib->end < aligned_end)`, and so, can be removed. The same is true for the condition: + if ((lib->exec_end == (uintptr_t)-1L) || (lib->exec_end < aligned_end)) { + lib->exec_end = aligned_end; + } `+ print_debug("%s [%d] 0x%lx-0x%lx: base = 0x%lx, vaddr = 0x%lx, memsz = 0x%lx, filesz = 0x%lx\n", lib->name, cnt, aligned_start, aligned_end, lib->base, ph->p_vaddr, ph->p_memsz, ph->p_filesz);` It is better to split this long line. Thanks, Serguei ------------- PR: https://git.openjdk.java.net/jdk/pull/2563
