diff mbox series

objdump: Round ASCII art lines in jump visualization

Message ID 878rbrgzoz.fsf@gmail.com
State Accepted
Headers
Series objdump: Round ASCII art lines in jump visualization |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Waqar Hameed July 7, 2023, 9:47 p.m. UTC 
  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(-)

Comments

Nick Clifton July 18, 2023, 3:24 p.m. UTC | #1 
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
Waqar Hameed July 18, 2023, 5:21 p.m. UTC | #2 
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.
Nick Clifton July 19, 2023, 10:18 a.m. UTC | #3 
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
diff mbox series

Patch

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;
 		}
 	    }