Hi, when running DhrakeParseClass, are you selecting the right address with your mouse cursor? It's supposed to be the start of the VMT address (labeled VMT_*_TSomething).

Is the data at address 004ab448 defined (created) as a function or is it a reference to one?

Can you please provide full Ghidra screenshots of the VMT_ in the Listing window and also one with data present at 004ab448? Feel free to hide anything sensitive.

If you search for the function in Data Type Manager, does it show up twice? e.g. FUN_004ab448 and a FUN_004ab448 *. Is the the former defined as a Function (prefixed with a "F")?

Are you running the latest version of Dhrake from this repo?

Lastly, do you happen to know the Delphi compiler version used for this executable?

Dhrake works fine with Ghidra 10.3 (tested on my side) so no worries there.

Hey @sarog, PR #8 looks good to me - but since you have so heavily contributed code I'd like to hear your opinion =).

Hey @sarog, PR #8 looks good to me - but since you have so heavily contributed code I'd like to hear your opinion =).

Indeed, I reviewed the change, but I believe it is not the full appropriate fix. Calling new PointerDataType() will create a new Pointer DT to the function. What if the reference already exists? If I am not mistaken, Ghidra automatically creates a pointer reference to a function when it's first defined, so creating another one with the same name will throw an exception.

This is what I recommend instead (it's existing code I haven't contributed yet) as it retrieves the function's existing pointer Data Type:

if (vtbl != null) {
    this.log(String.format("  Adding function definition %s::%s at %s to %s", className, name, entryPoint, vtbl.getName()));
    try {
        vtbl.add(this.getCurrentProgram().getDataTypeManager().getPointer(this.addFnType(function, catDefault)), 4, name, "");
    } catch (IllegalArgumentException e) {
        this.log("[ERROR] Cannot add function %s to %s", name, vtbl.getName());
    }
}

The catDefault argument of addFnType() can be omitted -- it's part of the category & subcategory creation I had mentioned a few months ago. :)

We can extend the code to include the creation of the Pointer DT in the event it is missing (re: PR #8) if the retrieval fails.

That said, I am waiting for @msm-cert to follow-up to fully understand the issue at hand. I've seen this problem before, and I suspect it's just bad disassembly by Ghidra getting in the way.

Hi! Thanks for your prompt response.

Let's start:

Hi, when running DhrakeParseClass, are you selecting the right address with your mouse cursor? It's supposed to be the start of the VMT address (labeled VMT_*_TSomething).

Yes.

Is the data at address 004ab448 defined (created) as a function or is it a reference to one?

If I understand your question correctly, it's a function (of type FUN_004ab448):

                             undefined FUN_004ab448()
             undefined         AL:1           <RETURN>
                             TFont::FUN_004ab448                             XREF[69]: ...
        004ab448 92              XCHG       EAX,EDX
        ....

TFont::FUN_004ab448

Can you please provide full Ghidra screenshots of the VMT_ in the Listing window and also one with data present at 004ab448? Feel free to hide anything sensitive.

Not sure it'll be helpful, since I've already ran my modified script on the binary so it may not be representative. I've created a fresh ghidra project to make this screen:

If you search for the function in Data Type Manager, does it show up twice? e.g. FUN_004ab448 and a FUN_004ab448 *. Is the the former defined as a Function (prefixed with a "F")?

Once, yes:

Are you running the latest version of Dhrake from this repo?

Yes, I've copied the files from the master branch to my ghidra_scripts directory.

Lastly, do you happen to know the Delphi compiler version used for this executable?

Possibly. Looking for strings in the compiled binary I see Embarcadero Delphi for Win32 compiler version 35.0 (28.0.42600.6491).

I've also used the build of IDR provided with this repository, if that helps.

Indeed, I reviewed the change, but I believe it is not the full appropriate fix.

I believe I'm a strong reverse engineer, but my knowledge of Ghidra API is severely lacking :) - so thanks for your suggestions. It's entirely possible my fix is suboptimal, I just did the simplest thing that solved the problem in my case.

This is what I recommend instead (it's existing code I haven't contributed yet) as it retrieves the function's existing pointer Data Type:

I confirm that works too. I can run this on a random VMT_ symbol in my project and it works:

DhrakeParseClass.java> Running...
DhrakeParseClass.java> [Dhrake] TDCP_rijndael
DhrakeParseClass.java> [Dhrake] Creating class TDCP_rijndael
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004ab448
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::FUN_004ab448 at 004ab448 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::TComponent.DefineProperties
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::TComponent.DefineProperties at 004bfd98 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::TPersistent.Assign
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::TPersistent.Assign at 004ab378 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004bf6b8
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::FUN_004bf6b8 at 004bf6b8 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::TComponent.Loaded
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::TComponent.Loaded at 004bfe90 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::TComponent.Notification
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::TComponent.Notification at 004bfd24 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::TComponent.ReadState
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::TComponent.ReadState at 004bfe9c to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004c0cd0
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::FUN_004c0cd0 at 004c0cd0 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004c0cd4
DhrakeParseClass.java> [Dhrake]   Adding function definition TDCP_rijndael::FUN_004c0cd4 at 004c0cd4 to TDCP_rijndaelVT
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004c0ab4
...

In comparison, when I run the original version from this repository (just vtbl.add(this.addFnType(function), 4, name, "");):

DhrakeParseClass.java> Running...
DhrakeParseClass.java> [Dhrake] TDCP_rijndael
DhrakeParseClass.java> [Dhrake] Creating class TDCP_rijndael
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004ab448
DhrakeParseClass.java> [Dhrake] Wtf java.lang.IllegalArgumentException: The "FUN_004ab448" data type is not allowed in a composite data type.
DhrakeParseClass.java> Finished!

(the additional log line is added by me: this.log("Wtf %s", e); in catch).

Perfect, thank you for the details!

The issue is that there was no actual function defined at address 004ab448 even though there's a label saying otherwise, so the Pointer DT to FUN_004ab448 was also missing as a result. The error message "The X data type is not allowed in a composite data type" is basically another of way of saying "I need a reference (pointer) instead, not a literal function" in a Ghidra Data Type.

Please go to address 004ab448 and press the F key to create a function in Ghidra's Listing. I imagine the appropriate pointer Data Type also gets created if the function is correctly defined. This leads me to ask the following question: did you run Ghidra's Auto Analysis (defaults are OK) before using Dhrake? :)

Are you sure that's it? This looks like a Ghidra function to me:

The error message "The X data type is not allowed in a composite data type" is basically another of way of saying "I need a reference (pointer) instead, not a literal function" in a Ghidra Data Type.

Yeah, I got it - that's why I've tried adding pointer type myself explicitly. I noticed this by trying to add a field to a structure with a type FUN_004ab448, and it was automatically converted by the UI to FUN_004ab448*.

This leads me to ask the following question: did you run Ghidra's Auto Analysis (defaults are OK) before using Dhrake? :)

Sure (even the aggresive opcode finder originally, without it for the second time) :). No point in working without it.

Let me try the whole process again from the start:

1. I load the binary to ghidra and run auto analysis
2. I execute DhrakeInit.java
3. I go to a random VMT symbol, for example VMT_5DDB0C_TDCP_rijndael at 005ddb0c
4. I run DhrakeParseClass on that symbol, It logs juts:

DhrakeParseClass.java> Running...
DhrakeParseClass.java> [Dhrake] TDCP_rijndael
DhrakeParseClass.java> [Dhrake] Creating class TDCP_rijndael
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004ab448
DhrakeParseClass.java> Finished!

5. Two structures are created: TDCP_rijndael (with just VT) and TDCP_rijndaelVT (empty)
6. I add a this.log("Wtf %s", e) to the catch clause, and run again. A familiar error appears:

DhrakeParseClass.java> Finished!
DhrakeParseClass.java> Running...
DhrakeParseClass.java> [Dhrake] TDCP_rijndael
DhrakeParseClass.java> [Dhrake] Creating class TDCP_rijndael
DhrakeParseClass.java> [Dhrake] adding function TDCP_rijndael::FUN_004ab448
DhrakeParseClass.java> [Dhrake] Wtf java.lang.IllegalArgumentException: The "FUN_004ab448" data type is not allowed in a composite data type.
DhrakeParseClass.java> Finished!

7. FUN_004ab448 walks like a function and quacks like a function:
   
8. I can manually add a field of that type to TDCP_rijndaelVT (data type changes to a function pointer automatically):
   

If that helps, I can share the sample I'm working on with you privately (it's not particularly sensitive, but I can't upload it publicly). Maybe you are able to reproduce it.

Are you sure that's it? This looks like a Ghidra function to me:

I wasn't questioning whether it was a function or not but rather if Ghidra had successfully defined it as such. I'm used to seeing the large "FUNCTION" comment in Ghidra hence why I asked for screenshots. :)

I'm mainly trying to figure out is why a pointer DT was not created for this function in the first place. I'm also wondering why I wrote the code above a few months ago to solve this particular issue, lol.

All that said, your PR is still warranted as it fixes this issue. You are welcome to use the code I wrote above in your submission to avoid duplicates. Of course, @huettenhain has the final say in this matter. 😁

If that helps, I can share the sample I'm working on with you privately (it's not particularly sensitive, but I can't upload it publicly). Maybe you are able to reproduce it.

I'm ready to check it out (for science!) if you are willing to share!

I wasn't questioning whether it was a function or not but rather if Ghidra had successfully defined it as such. I'm used to seeing the large "FUNCTION" comment in Ghidra hence why I asked for screenshots. :)

Yeah, sorry for cropping my original screenshot a bit too much.

All that said, your PR is still warranted as it fixes this issue. You are welcome to use the code I wrote above in your submission to avoid duplicates. Of course, @huettenhain has the final say in this matter. 😁

I'll check your code and integrate it into my PR. I'm a bit short on time today, so it'll probably wait until Monday (or I'll commit it over the weekend).

I'm ready to check it out (for science!) if you are willing to share!

Can I have your email address (or another contact method)? I don't see it on your profile. You can also email me at [email protected].

Just noting that I am all 👍 on this conversation. By this point, @sarog probably knows the code better than I do 😬.