Techdaan / rs3nxt-ghidra-scripts

Ghidra scripts to help reverse-engineer RS3's NXT client
GNU General Public License v3.0
13 stars 4 forks source link

Fixes the script to run 921-4 #4

Closed AridTag closed 2 years ago

AridTag commented 2 years ago

Jag have added a new implementation of HeapInterface and a wrapper function that gets called instead of calling HeapInterface::Alloc directly

longlong jag::HeapAlloc(longlong num_bytes,longlong alignment)

{
  longlong lVar1;

  lVar1 = HeapInterfaceNew::Alloc(HeapInterfaceNew::g_pHeapInterfaceNew,num_bytes,alignment);
  if (lVar1 == 0) {
    FUN_140594cf0(&DAT_1408c537c,"D:/bb-b-01/NXT-BUIL4695-BWXR/libs/heap/NewCustom.cpp",0x69,0,
                  "Failed to allocate %llu bytes with alignment %llu",(char)num_bytes);
  }
  return lVar1;
}
Techdaan commented 2 years ago

Awesome to see someone pushed a fix for this :) I had something ready but haven't pushed myself yet.

Are you sure there is a whole new implementation? It might be better to register the wrapper function as jag::HeapInterface::CheckedAlloc, since it seems to only add error checking.

AridTag commented 2 years ago

No, I'm not positive that it's a whole new implementation. The inclusion of a debug message referencing NewCustom.cpp leads me to believe they are working on a new HeapInterface implementation to replace what was there before. And adding a new global function to be called so that the inner implementation can be swapped out as needed without having to change a bunch of code, which I imagine they had to do to implement this.

I could certainly change it to register as that if you would prefer it doesn't much matter to me

AridTag commented 2 years ago

I should also note, that I had renamed HeapInterface to HeapInterfaceNew building on my assumption. But obviously I can roll that back as well

Techdaan commented 2 years ago

Ah that makes sense, yeah. I think for now keeping it consistent with the other naming would be good, to avoid having a lot of random names. Both are related to the same so it should be fine.

If you have a different opinion/disagree please let me know :)

AridTag commented 2 years ago

You got it boss. Renamed to CheckedAlloc and undid other renames.

AridTag commented 2 years ago

Oh, yikes. I used intellij to format the document. That's a huge diff. Sorry about that. Let me fix that