n-o-o-n / idp_hexagon

Hexagon processor module for IDA Pro disassembler
GNU Lesser General Public License v3.0
97 stars 22 forks source link

Updated to SDK 7.6 #11

Closed flowswitch closed 2 years ago

flowswitch commented 2 years ago

I've updated the code to run on v7.6 without VMT hacks (recreated the missing proc_def_t, subclassed the procmod_t etc) while trying to preserve the older version compatibility. The code builds with 7.6, 7.5 and 7.2 SDKs on Linux, but I don't have a 7.2 for testing. Perhaps someone could test the compatibility before merging this. Tested to run on 7.6 Linux for now.

n-o-o-n commented 2 years ago

The latest commit adds support for IDA v7.6, many thanks to @flowswitch!

Regarding the VMT hack: First of all, the proposed code differs from elf.dll in subtle, but important details. Secondly, I don't want to support changes made inside the IDA in my code. For example, in IDA v7.6 function proc_def_t::proc_adjust_entry was modified comparing to v7.5. Thirdly, in IDA v7.7 the implementation of proc_def_t has finally been opened.

Regarding asserts. If you have an input file on which the module generates an assert, please share it with me.

flowswitch commented 2 years ago

asserts

I was getting asserts from ana.cpp:new_value(): if (!hvx) { assert( (nt & 1) == 0 ); After converting asserts to warnings I've inspected offending addresses and found data items with xrefs from some code. Looks like IDA just tried to follow a xref in code mode and called ana() on data bytes matching some "new value" insn, but having an odd nt field. This is not an idp bug but a wrong input, no need to crash, just return 0 from ana().

VMT hacks

It crashes on a Linux version (different object layout?), @bkerler's fork has the same problem IIRC. But you can just fast forward to 7.7 of course, supporting every SDK version would be annoying.

bkerler commented 2 years ago

@n-o-o-n here you go, crashes all the time: https://drive.google.com/file/d/1VmfhabGTEc1aq-X-hNWcB4XXTU3i6PX9/view?usp=sharing

@flowswitch returning 0 doesn't seem to solve the issue. I've forwarded it the module to ida 7.7 for tests.

bkerler commented 2 years ago

Disabling "Create offset if data xref to seg32 exists" in Kernel analysis options 2 seems to prevent the crash.

flowswitch commented 2 years ago

@bkerler confirming the crash after some 1-3 minutes autoanalysis with your modem, but this one is not related to asserts (although with asserts the same file would crash IDA immediately, hitting the same (nt & 1)==0 test). The offending code is this: _ea_t start = getseg( ea )->start_ea, end = find_packetend( ea ); in find_packet_boundaries(). getseg() returning null I suspect. Update: it fails doing getseg(0x18EB9D8) Update2: looks like the last dword of segment at 0x18EB9D4 gets treated as a const extender, so ana() steps to the next address.

flowswitch commented 2 years ago

Fix:

find_packet_boundaries():
    auto seg = getseg( ea );
    if (!seg)
    {
        return BADADDR;
    }
    ea_t start = seg->start_ea, end = find_packet_end( ea );

ana():
    s_pkt_start = find_packet_boundaries( ea, &pkt_end );
    if (s_pkt_start==BADADDR)
    {
        return 0;
    }
bkerler commented 2 years ago

Cool, thx. Will test it. How did you debug this ?

flowswitch commented 2 years ago

$ gdb —args /path/to/ida modem.elf gdb> run

n-o-o-n commented 2 years ago

The latest commit fixes two crashes - one reported by @bkerler and another due to my stupidity. My deepest thanks to @flowswitch.