On Fri, 23 Jan 2026 17:33:59 GMT, Chris Plummer <[email protected]> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add is_in()
>
> src/jdk.hotspot.agent/linux/native/libsaproc/symtab.c line 555:
>
>> 553: static bool is_in(uintptr_t offset, struct elf_symbol* sym) {
>> 554: if (offset == sym->offset) {
>> 555: // offset points the top of the symbol
>
> Suggestion:
>
> // offset points to the top of the symbol
This code is now confusing to read without knowledge that it is trying to solve
the issue of a 0 sized symbol. I would suggest having it check `(sym->size == 0
&& offset == sym->offset)`, and add a comment explaining why, and then the
check below should be reverted back to `offset >= 0` instead of `offset > 0`.
It is confusing to not have it check the full address range, especially since
the comment indicates that it does.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29023#discussion_r2722233077
