New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diff view for disassembler output #544

Open
adamsitnik opened this Issue Sep 10, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@adamsitnik
Member

adamsitnik commented Sep 10, 2017

From private conversation with @JosephTremoulet

trying to replicate (in some fashion) the various rewrites we apply under COMPlus_JitDiffableDisasm, so that a straightforward textual diff identifies significant code changes and not simply things getting loaded at different addresses, etc.
some sort of diff view integrated into the html like code reviews on GitHub etc.

Imho we should print the disasm in some smarter way and use some js library for having nice visualization of the diff

@rolshevsky

This comment has been minimized.

Show comment
Hide comment
@rolshevsky

rolshevsky Nov 6, 2017

Collaborator

@adamsitnik @AndreyAkinshin Hello,
Can you guys please assign me on this issue? I'm gonna try to implement it.

Collaborator

rolshevsky commented Nov 6, 2017

@adamsitnik @AndreyAkinshin Hello,
Can you guys please assign me on this issue? I'm gonna try to implement it.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Nov 6, 2017

Member

@rolshevsky awesome!

@AndreyAkinshin could you assign @rolshevsky ?

Member

adamsitnik commented Nov 6, 2017

@rolshevsky awesome!

@AndreyAkinshin could you assign @rolshevsky ?

@adamsitnik adamsitnik removed the up-for-grabs label Nov 6, 2017

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Nov 6, 2017

Member

@rolshevsky, I added you to the collaborators list. Please, accept the invitation, then I will be able to assign.

Member

AndreyAkinshin commented Nov 6, 2017

@rolshevsky, I added you to the collaborators list. Please, accept the invitation, then I will be able to assign.

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin
Member

AndreyAkinshin commented Nov 6, 2017

@rolshevsky, done.

@rolshevsky

This comment has been minimized.

Show comment
Hide comment
@rolshevsky
Collaborator

rolshevsky commented Nov 6, 2017

@rolshevsky

This comment has been minimized.

Show comment
Hide comment
@rolshevsky

rolshevsky Dec 5, 2017

Collaborator

Hi @adamsitnik,

Currently our Asm class contains text representation of asm instruction with instruction address:

0517180e 40              inc     eax
0517180f b964000000      mov     ecx,64h 

Is it possible to expand this class with additional property which will be contain instruction without address?

inc     eax
mov     ecx,64h

Using this we can implement 2 modes for showing diffs in our DiffView (with asm instruction address and without).

What do you think about it? Does it make sense?

Collaborator

rolshevsky commented Dec 5, 2017

Hi @adamsitnik,

Currently our Asm class contains text representation of asm instruction with instruction address:

0517180e 40              inc     eax
0517180f b964000000      mov     ecx,64h 

Is it possible to expand this class with additional property which will be contain instruction without address?

inc     eax
mov     ecx,64h

Using this we can implement 2 modes for showing diffs in our DiffView (with asm instruction address and without).

What do you think about it? Does it make sense?

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 5, 2017

Member

@rolshevsky it's a very good idea!

I believe that we should implement #546 first. So the view would be even more user friendly. Example:

HasFlagBench.Bench.HasFlag():
LO:
    xor     eax,eax
    mov     edx,dword ptr [rcx+8]
L1:
    mov     r8d,edx
    and     r8d,0Ch
    cmp     r8d,0Ch
    sete    r8b
    mov     byte ptr [rcx+0Ch],r8b
    inc     eax
    cmp     eax,3E8h
    jl      L1
L2:
    ret
Member

adamsitnik commented Dec 5, 2017

@rolshevsky it's a very good idea!

I believe that we should implement #546 first. So the view would be even more user friendly. Example:

HasFlagBench.Bench.HasFlag():
LO:
    xor     eax,eax
    mov     edx,dword ptr [rcx+8]
L1:
    mov     r8d,edx
    and     r8d,0Ch
    cmp     r8d,0Ch
    sete    r8b
    mov     byte ptr [rcx+0Ch],r8b
    inc     eax
    cmp     eax,3E8h
    jl      L1
L2:
    ret
@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 7, 2017

Member

@rolshevsky I implemented #546. Please take a look at DisassemblyPrettifier.Prettify. It looks much better now and you should be able to compare same asm even if addresses change.

image

Member

adamsitnik commented Dec 7, 2017

@rolshevsky I implemented #546. Please take a look at DisassemblyPrettifier.Prettify. It looks much better now and you should be able to compare same asm even if addresses change.

image

@AndreyAkinshin AndreyAkinshin modified the milestones: v0.10.x, v0.11.x Apr 9, 2018

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Oct 19, 2018

Member

No updates from @rolshevsky for almost a year, I make the issue up-for-grabs again.

Member

adamsitnik commented Oct 19, 2018

No updates from @rolshevsky for almost a year, I make the issue up-for-grabs again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment