joxeankoret / diaphora

Diaphora, the most advanced Free and Open Source program diffing tool.
http://diaphora.re
GNU Affero General Public License v3.0
3.62k stars 373 forks source link

Importing info results in incorrect label and comment placement #97

Closed ZakDanger closed 6 years ago

ZakDanger commented 6 years ago

I did a diff between two versions of a x64 freebsd kernel. When it completed I selected and imported just 1 function from the Best Matches page where the match had been due to function name. I knew that the matched function was correct as it was named correctly in both instances.

You can see below the results of the import operation. On the left side is the import destination. On the right side is the import source.

Results of import

The functions are a little bit different, as to be expected when diffing. The imported label and comment are placed at incorrect locations.

Is this the expected results? I would have thought it'd at least get the subroutine call naming correct?!

joxeankoret commented 6 years ago

It's actually a bug. Thanks for reporting! You're not the only one having problems when importing instruction level symbols :/

leoetlino commented 6 years ago

I think this is a duplicate of https://github.com/joxeankoret/diaphora/issues/84 (since both issues are similar and have to do with import_instruction_level). For what it's worth, @AGG2017's branch fixes the issue for me.

joxeankoret commented 6 years ago

Yes, the branch from @AGG2017 partially fixes it, but a proper fix needs more work. Use it meanwhile, as until 14th December I will not have time, I'm afraid.

joxeankoret commented 6 years ago

I believe I have a good patch for it that I have been coding during these days. If someone could send me binary files and/or databases where this behaviour can be reproduced so I can test my patches, I would be very thankful.

leoetlino commented 6 years ago

@joxeankoret sorry, I missed the email notification at first! Do you still need a test database?

joxeankoret commented 6 years ago

Yes, even when it's fixed, so I can verify that whenever I change something, my new changes doesn't break what was working before.

leoetlino commented 6 years ago

Library (which contains tons of symbols): https://f.leolam.fr/diaphora/libogc.o Exported database: https://f.leolam.fr/diaphora/libogc.sqlite (generated with 39c81f6) The issue with older versions can be easily reproduced with some symbols, so the database only includes a subset of functions (000487A0 to 0004C538). And here is the binary: https://f.leolam.fr/diaphora/stage1.elf

When you diff, Diaphora will recognise a function called STM_SetLedMode. It's supposed to look like this:

STM_RebootSystem proc near
    ...
    lis       r5, __stm_immbufin@ha
    li    r0, 0
    lis       r7, __stm_immbufout@ha
    lwz       r3, __stm_imm_fd-0x80000# Relocation out of range
    stw       r0, __stm_immbufin@l(r5)
    li    r4, 0x2001# ioctl
    addi      r5, r5, __stm_immbufin@l# buffer_in
    li        r6, 0x20 # ' '# len_in
    addi      r7, r7, __stm_immbufout@l# buffer_io
    li        r8, 0x20 # ' '# len_io
    bl        IOS_Ioctl
   ...

After importing, you end up (if it's broken, like with 39c81f6) with something like this:

.text0:8001376C                 lis       r5, dword_80050BE0@ha
.text0:80013770                 li        r0, 0
.text0:80013774                 lis       r7, -0x7FFB
.text0:80013778                 lwz       r3, 0x44(r13)
.text0:8001377C                 stw       r0, dword_80050BE0@l(r5)
.text0:80013780                 li        r4, 0x2001
.text0:80013784                 addi      r5, r5, dword_80050BE0@l # ioctl
.text0:80013788                 li        r6, 0x20
.text0:8001378C                 addi      r7, r7, 0xC00 # 0x80050C00
.text0:80013790                 li        r8, 0x20
.text0:80013794                 bl        sub_80011FE0

Note the missing comments, variable and function names. There's one comment (ioctl) but it's at the wrong location -- it should be next to the li r4, 0x2001 line.

Hope this helps!

joxeankoret commented 6 years ago

I can see at least 2 problems here. Thanks for reporting and thanks a lot for sharing binaries! PS: I'm currently sick so I will take some time to fix.

joxeankoret commented 6 years ago

Fixed with https://github.com/joxeankoret/diaphora/commit/514937898beee605d4454096cae7e124d181e0dc

ZakDanger commented 6 years ago

I gave this a quick test and it appears ok. I will continue testing and report if I still have issues.

Is there some way I can contact you to share private binaries for testing?

joxeankoret commented 6 years ago

Sure! Check this page for contact information: http://joxeankoret.com/contact.html