[PATCH] objdump: Round ASCII art lines in jump visualization

[PATCH] objdump: Round ASCII art lines in jump visualization

[Waqar Hameed]

Lines with rounded corners are easier to follow (and easier on the
eyes). Use `,` and `'` instead of `/` and `\`, respectively, when
drawing lines for jumps.

Before:

    <debug_get_target_type>:
                         endbr64
                         test   %rsi,%rsi
            /----------- je     <debug_get_target_type+0x48>
            |            sub    $0x8,%rsp
            |            xor    %edx,%edx
            |            call   <debug_get_real_type>
            |            test   %rax,%rax
            |  /-------- je     <debug_get_target_type+0x3d>
            |  |         mov    (%rax),%edx
            |  |         cmp    $0x14,%edx
            |  |  /----- je     <debug_get_target_type+0x2c>
            |  |  |  /-- ja     <debug_get_target_type+0x38>
            |  |  |  |   cmp    $0xc,%edx
            |  |  +--|-- je     <debug_get_target_type+0x2c>
            |  |  |  |   cmp    $0xe,%edx
            |  +--|--|-- jne    <debug_get_target_type+0x3d>
            |  |  >--|-> mov    0x18(%rax),%rax
            |  |  |  |   add    $0x8,%rsp
            |  |  |  |   ret
            |  |  |  |   nopl   (%rax)
            |  |  |  \-> cmp    $0x15,%edx
            |  |  \----- je     <debug_get_target_type+0x2c>
            |  \-------> xor    %eax,%eax
            |            add    $0x8,%rsp
            |            ret
            |            nopl   0x0(%rax)
            \----------> xor    %eax,%eax
                         ret
                         nopl   0x0(%rax,%rax,1)

After:

    <debug_get_target_type>:
                         endbr64
                         test   %rsi,%rsi
            ,----------- je     <debug_get_target_type+0x48>
            |            sub    $0x8,%rsp
            |            xor    %edx,%edx
            |            call   <debug_get_real_type>
            |            test   %rax,%rax
            |  ,-------- je     <debug_get_target_type+0x3d>
            |  |         mov    (%rax),%edx
            |  |         cmp    $0x14,%edx
            |  |  ,----- je     <debug_get_target_type+0x2c>
            |  |  |  ,-- ja     <debug_get_target_type+0x38>
            |  |  |  |   cmp    $0xc,%edx
            |  |  +--|-- je     <debug_get_target_type+0x2c>
            |  |  |  |   cmp    $0xe,%edx
            |  +--|--|-- jne    <debug_get_target_type+0x3d>
            |  |  >--|-> mov    0x18(%rax),%rax
            |  |  |  |   add    $0x8,%rsp
            |  |  |  |   ret
            |  |  |  |   nopl   (%rax)
            |  |  |  '-> cmp    $0x15,%edx
            |  |  '----- je     <debug_get_target_type+0x2c>
            |  '-------> xor    %eax,%eax
            |            add    $0x8,%rsp
            |            ret
            |            nopl   0x0(%rax)
            '----------> xor    %eax,%eax
                         ret
                         nopl   0x0(%rax,%rax,1)

Signed-off-by: Waqar Hameed <whame91@gmail.com>
---
 binutils/objdump.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/binutils/objdump.c b/binutils/objdump.c
index a35982ea969..ca4813ce872 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -2873,10 +2873,10 @@ jump_info_visualize_address (bfd_vma address,
 		{
 		  if (address <= ji->end)
 		    line_buffer[offset] =
-		      (jump_info_min_address (ji) == address) ? '/': '+';
+		      (jump_info_min_address (ji) == address) ? ',': '+';
 		  else
 		    line_buffer[offset] =
-		      (jump_info_max_address (ji) == address) ? '\\': '+';
+		      (jump_info_max_address (ji) == address) ? '\'': '+';
 		  color_buffer[offset] = color;
 		}
 	    }
@@ -2907,9 +2907,9 @@ jump_info_visualize_address (bfd_vma address,
 		{
 		  if (jump_info_min_address (ji) < address)
 		    line_buffer[offset] =
-		      (jump_info_max_address (ji) > address) ? '>' : '\\';
+		      (jump_info_max_address (ji) > address) ? '>' : '\'';
 		  else
-		    line_buffer[offset] = '/';
+		    line_buffer[offset] = ',';
 		  color_buffer[offset] = color;
 		}
 	    }
-- 
2.34.1

Re: [PATCH] objdump: Round ASCII art lines in jump visualization

[Nick Clifton]

Hi Waqar,

> Lines with rounded corners are easier to follow (and easier on the
> eyes). Use `,` and `'` instead of `/` and `\`, respectively, when
> drawing lines for jumps.

Well this is kind of a personal thing, and not everyone may agree with
your choice of characters.  So how about allowing the user to select the
characters that they like ?  For example if you added a new command line
option:

   --visualize-jump-chars="/,+>'"

which could then be used to provide the characters displayed at various
points on the graph.  Obviously this would be a more complicated patch,
and would require adding some more documentation, but overall I think that
it would be worthwhile...

Cheers
   Nick

Re: [PATCH] objdump: Round ASCII art lines in jump visualization

[Waqar Hameed]

On Tue, Jul 18, 2023 at 16:24 Nick Clifton <nickc@redhat.com> wrote:

>> Lines with rounded corners are easier to follow (and easier on the
>> eyes). Use `,` and `'` instead of `/` and `\`, respectively, when
>> drawing lines for jumps.
>
> Well this is kind of a personal thing, and not everyone may agree with
> your choice of characters.  So how about allowing the user to select the
> characters that they like ?  For example if you added a new command line
> option:
>
>   --visualize-jump-chars="/,+>'"
>
> which could then be used to provide the characters displayed at various
> points on the graph.  Obviously this would be a more complicated patch,
> and would require adding some more documentation, but overall I think that
> it would be worthwhile...

I understand that this is highly subjective and not everyone would agree
with the argument stated in the commit message (I guess the "scientific
method" would be to do a survey :) ).

I don't think an option that specifies characters would make much sense;
there is only a small subset of characters that qualifies to represent
lines. An option to choose "ASCII style" is probably better, e.g.
`--visualize-jumps=round` or something.

However, I'm OK with just dropping this patch. It's not really a big
deal.

Re: [PATCH] objdump: Round ASCII art lines in jump visualization

[Nick Clifton]

Hi Waqar,

> However, I'm OK with just dropping this patch. It's not really a big
> deal.

OK, well then lets keep things simple and leave them as they are.  But
thank you for submitting the patch, and please feel free to submit more
patches in the future.

Cheers
   Nick

Re: [PATCH] objdump: Round ASCII art lines in jump visualization

[Nick Clifton]

Hi Waqar,

> Lines with rounded corners are easier to follow (and easier on the
> eyes). Use `,` and `'` instead of `/` and `\`, respectively, when
> drawing lines for jumps.

Approved and applied.

Cheers
   Nick

Re: [PATCH] objdump: Round ASCII art lines in jump visualization

[Waqar Hameed]

On Tue, Apr 23, 2024 at 16:11 +0100 Nick Clifton <nickc@redhat.com> wrote:

[...]

> Approved and applied.

That was kind of unexpected after 9 months. Glad of the outcome though :)