RFR: 8337662: Improve os::print_hex_dump for printing Instructions sections [v6]

[Thomas Stuefe]

On Mon, 19 Aug 2024 13:26:22 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> Currently we use os::print_hex_dump for printing Instruction sections in the hserr file. The function could be slightly improved, .e.g. by showing directly the pc .
>> The instructions section would for example look like this, with a small helper '=>' pointing to the pc. Makes the output more readable.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove extra whitespace

Hi Matthias, 

sorry for the delay.

Cheers, Thomas

src/hotspot/share/runtime/os.cpp line 1028:

> 1026:     assert(highlight_address >= start && highlight_address < end,
> 1027:            "address %p to highlight not in range %p - %p", highlight_address, start, end);
> 1028:   }

Can be shortened a bit
Suggestion:

  assert(highlight_address == nullptr || (highlight_address >= start && highlight_address < end),
         "address %p to highlight not in range %p - %p", highlight_address, start, end);

src/hotspot/share/runtime/os.cpp line 1047:

> 1045:       // highlight start of line if address of interest is located in the line
> 1046:       bool should_highlight = false;
> 1047:       if (highlight_address >= p && highlight_address < p + cols_per_line) should_highlight = true;

Can be shortened, and should use the byte line size, not the size in printed units.

Suggestion:

      const bool should_highlight = (highlight_address >= p && highlight_address < p + bytes_per_line);


This highlights a gap in your testing. You should also test with unitsizes other than 1.

test/hotspot/gtest/runtime/test_os.cpp line 278:

> 276:   do_test_print_hex_dump_highlighted(from, to, 1, 32, logical_start, PAT_HL_1B, from+32);
> 277:   do_test_print_hex_dump_highlighted(from, to, 1, 32, logical_start, PAT_HL_1B, from+60);
> 278: 

Please add at least one test with a unit size > 1.

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

Changes requested by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/20419#pullrequestreview-2250099803
PR Review Comment: https://git.openjdk.org/jdk/pull/20419#discussion_r1724625737
PR Review Comment: https://git.openjdk.org/jdk/pull/20419#discussion_r1724628996
PR Review Comment: https://git.openjdk.org/jdk/pull/20419#discussion_r1724634167