On Thu, 15 Oct 2026 22:31:10 GMT, Chris Plummer <[email protected]> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Skip tests on musl c on AArch64
>
> src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.cpp line 81:
>
>> 79: uint64_t length = *(reinterpret_cast<uint32_t *>(_buf));
>> 80: _buf += 4;
>> 81: if (!_lib->frame.is_debug_frame && length == 0xffffffff) {
>
> I believe this check is incorrect. .debug_frame still uses 0xffffffff to
> indicate that a 64-bit length follows.
You are right. Fixed.
> src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.cpp line 102:
>
>> 100: unsigned char *end = _buf + length;
>> 101:
>> 102: _buf += 4; // Skip ID (This value of CIE would be always 0 in
>> .eh_frame / 0xffffffff in .debug_frame)
>
> But this isn't true of DWARF64, which you seem to be partially supporting on
> lines 81-83 above.
Fixed.
> src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.cpp line 103:
>
>> 101:
>> 102: _buf += 4; // Skip ID (This value of CIE would be always 0 in
>> .eh_frame / 0xffffffff in .debug_frame)
>> 103: _buf++; // Skip version
>
> This will break with version 4 CIE because it adds additional info at the
> start. Will this ever be an issue for us?
Updated to check DWARF version and skip additional fields since DWARF 4
appropriately if needs.
> src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.cpp line 348:
>
>> 346: while (_buf <= end) {
>> 347: uint64_t length = get_entry_length();
>> 348: if (length == 0L) {
>
> If this is a .debug_frame, it won't end with a zero-length terminator. You
> need to do bounds checks to make sure you are not reading past the end of the
> buffer.
Fixed.
> src/jdk.hotspot.agent/linux/native/libsaproc/dwarf.cpp line 354:
>
>> 352: unsigned char *start_of_entry = _buf;
>> 353: uint32_t id = *(reinterpret_cast<uint32_t *>(_buf));
>> 354: _buf += 4;
>
> But this isn't 4 bytes for DWARF64, which you seem to be partially supporting
> on lines 81-83 above.
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/33375#discussion_r4292345483
PR Review Comment: https://git.openjdk.org/jdk/pull/33375#discussion_r4292348477
PR Review Comment: https://git.openjdk.org/jdk/pull/33375#discussion_r4292340727
PR Review Comment: https://git.openjdk.org/jdk/pull/33375#discussion_r4292329039
PR Review Comment: https://git.openjdk.org/jdk/pull/33375#discussion_r4292386494
