`RecoverClassesFromRTTIScript` fails for Windows PE files if run after certain manual updates have been made (see description)

[Wall-AF]

Describe the bug When running script RecoverClassesFromRTTIScript on a Windows PE DLL having previously performed some manual updates involving class/vftable (struct's) creation etc, setting member functions to __stdcall (because the current __thiscall prototype used a register and these particular functions stored their this pointer on the stack (how dare they!)) and naming that parameter "this", the script fails saying: > Error running script: RecoverClassesFromRTTIScript.java Not very informative!

Digging into it, the problem is twofold:

1. two (or more) "this" prototypes are needed (plus the headache of enabliing Ghidra to try each in turn when assigning them (maybe doable); and
2. without 1), the error is caused by: https://github.com/NationalSecurityAgency/ghidra/blob/fd2dde2608f45ba5ecca11ab3081454fed47ba34/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/VariableSymbolDB.java#L188-L189

To Reproduce Steps to reproduce the behavior: See description above.

Expected behavior The script should:

1. not just fail without adequate information to ensure the user can fix the reason (in this case, the use of the parameter name "this" without a __thiscall calling convention as the script is trying to update to) and can then rerun it; or
2. allow a usable name when returning from method doGetName in class VariableSymbolDB.

Option 2 may have other consequencies!

Screenshots N/A.

Attachments N/A.

Environment (please complete the following information):

• OS: Windows 11
• Java Version: Temurin-21.0.3
• Ghidra Version: 11.4-DEV
• Ghidra Origin: locally built

Additional context Related to question posed in https://github.com/NationalSecurityAgency/ghidra/discussions/8163.

[Wall-AF]

There are lots of points in Ghidra's Java code that check the calling convention is __thiscall. If you add a prototype for another thiscall, say using different parameter options, your new one won't be found. This is because currrently the test is based on the conventions name and uses .equals(...).

There are a few solutions to this:

1. replace all .equals with .startsWith (and switch some of the comparitors around) - will work, but all must start with __thiscall.
2. use the attribute hasThis - probably involves more work, but should be better long term.

[ghidra007]

@Wall-AF Regarding the RTTI script, it says in the description that it should be run on freshly analyzed binary and also says it is very prototype:

"NOTE: For likely the best results, run this script on freshly analyzed programs. No testing has been done on user marked-up programs." and "PROTOTYPE Script to recover class information using RTTI structures."

I would recommend waiting until after the script is run to change anything. If you can point me at an example and tell me what you changed before running the script, I can at least try to make the error more informative. At this point I don't know if it is the script api that is outputting that error that or if it is the script doing that. (Upated: It is the script api - see comment below)

Regarding your two suggestions, are you asking for these changes across all of Ghidra or just the script?

Regarding your suggestions and just the script, I can try to take a look to see if doing either would be safe to do. It may do bad things if the prototypes are not similar enough and it makes assumptions based on that one prototype. Again, if you make the changes to your program afterwards, that would be safer. You could possibly write a script to do that since it would be burdensome to do so manually. Without seeing your example or one like it, I really wouldn't know for sure.

I am in the process of refactoring the script and at some point it will be turned into an analyzer so will happen before people are able to make changes to the program. This is why the script added that limitation in the documentation.

[ghidra007]

@Wall-AF I just took a quick look to see where that error is coming from and it is from the RunScriptTask.java when the script that it is running throws an exception. It looks like it also prints the stack trace to the console. If you have that, can you include it here? That way I can at least check for that exception and handle it more gracefully. Thanks!

[Wall-AF]

@ghidra007 I'll answer your points in turn below:;

1. I didn't see the script until I had already started looking/working on it, hence my initial changes! (In that case, as there is a caveat, this is user error!)
2. as this is a current'ish part of an app, I can't share my gzf here, but if you have a private share somewhere, I could copy it there. (The gzf I have has my few changes that you could repeatedly rerun the recovery script on.)
3. my two suggestion where not confined to this script set.
4. below is the stack trace (taken from Eclipse's Console):

DEBUG Analysis worker completed (RecoverClassesFromRTTIScript.java): class ghidra.app.script.GhidraScript$1   (AutoAnalysisManager.java:1376) 
ERROR Unexpected Exception: null java.lang.reflect.InvocationTargetException
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1705)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisTaskWrapper.run(AutoAnalysisManager.java:660)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:760)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:639)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:604)
	at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:55)
	at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:1)
	at ghidra.framework.plugintool.mgr.BackgroundCommandTask.run(BackgroundCommandTask.java:103)
	at ghidra.framework.plugintool.mgr.ToolTaskManager.run(ToolTaskManager.java:351)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: ghidra.util.exception.InvalidInputException: Symbol name contains invalid characters: [Invalid VariableSymbol - Deleted!]
	at ghidra.program.model.symbol.SymbolUtilities.validateName(SymbolUtilities.java:238)
	at ghidra.program.database.symbol.SymbolDB.doSetNameAndNamespace(SymbolDB.java:606)
	at ghidra.program.database.symbol.SymbolDB.setNameAndNamespace(SymbolDB.java:674)
	at ghidra.program.database.symbol.SymbolDB.setName(SymbolDB.java:540)
	at ghidra.program.database.function.VariableDB.setName(VariableDB.java:174)
	at ghidra.program.database.function.FunctionDB.updateFunction(FunctionDB.java:1451)
	at ghidra.program.database.function.FunctionDB.updateFunction(FunctionDB.java:1311)
	at classrecovery.RecoveredClassHelper.makeFunctionThiscall(RecoveredClassHelper.java:2277)
	at classrecovery.RecoveredClassHelper.nameVfunctions(RecoveredClassHelper.java:4764)
	at classrecovery.RecoveredClassHelper.fillInAndApplyVftableStructAndNameVfunctions(RecoveredClassHelper.java:4411)
	at classrecovery.RTTIWindowsClassRecoverer.processDataTypes(RTTIWindowsClassRecoverer.java:2201)
	at classrecovery.RTTIWindowsClassRecoverer.createAndApplyClassStructures(RTTIWindowsClassRecoverer.java:2136)
	at classrecovery.RTTIWindowsClassRecoverer.createRecoveredClasses(RTTIWindowsClassRecoverer.java:176)
	at RecoverClassesFromRTTIScript.run(RecoverClassesFromRTTIScript.java:307)
	at ghidra.app.script.GhidraScript.executeNormal(GhidraScript.java:405)
	at ghidra.app.script.GhidraScript$1.analysisWorkerCallback(GhidraScript.java:387)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1699)
	... 10 more
  (ConsoleComponentProvider.java:214) 
ERROR RecoverClassesFromRTTIScript.java: Error running script: RecoverClassesFromRTTIScript.java
java.lang.reflect.InvocationTargetException: null java.lang.reflect.InvocationTargetException
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1705)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisTaskWrapper.run(AutoAnalysisManager.java:660)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:760)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:639)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager.startAnalysis(AutoAnalysisManager.java:604)
	at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:55)
	at ghidra.app.plugin.core.analysis.AnalysisBackgroundCommand.applyTo(AnalysisBackgroundCommand.java:1)
	at ghidra.framework.plugintool.mgr.BackgroundCommandTask.run(BackgroundCommandTask.java:103)
	at ghidra.framework.plugintool.mgr.ToolTaskManager.run(ToolTaskManager.java:351)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: ghidra.util.exception.InvalidInputException: Symbol name contains invalid characters: [Invalid VariableSymbol - Deleted!]
	at ghidra.program.model.symbol.SymbolUtilities.validateName(SymbolUtilities.java:238)
	at ghidra.program.database.symbol.SymbolDB.doSetNameAndNamespace(SymbolDB.java:606)
	at ghidra.program.database.symbol.SymbolDB.setNameAndNamespace(SymbolDB.java:674)
	at ghidra.program.database.symbol.SymbolDB.setName(SymbolDB.java:540)
	at ghidra.program.database.function.VariableDB.setName(VariableDB.java:174)
	at ghidra.program.database.function.FunctionDB.updateFunction(FunctionDB.java:1451)
	at ghidra.program.database.function.FunctionDB.updateFunction(FunctionDB.java:1311)
	at classrecovery.RecoveredClassHelper.makeFunctionThiscall(RecoveredClassHelper.java:2277)
	at classrecovery.RecoveredClassHelper.nameVfunctions(RecoveredClassHelper.java:4764)
	at classrecovery.RecoveredClassHelper.fillInAndApplyVftableStructAndNameVfunctions(RecoveredClassHelper.java:4411)
	at classrecovery.RTTIWindowsClassRecoverer.processDataTypes(RTTIWindowsClassRecoverer.java:2201)
	at classrecovery.RTTIWindowsClassRecoverer.createAndApplyClassStructures(RTTIWindowsClassRecoverer.java:2136)
	at classrecovery.RTTIWindowsClassRecoverer.createRecoveredClasses(RTTIWindowsClassRecoverer.java:176)
	at RecoverClassesFromRTTIScript.run(RecoverClassesFromRTTIScript.java:307)
	at ghidra.app.script.GhidraScript.executeNormal(GhidraScript.java:405)
	at ghidra.app.script.GhidraScript$1.analysisWorkerCallback(GhidraScript.java:387)
	at ghidra.app.plugin.core.analysis.AutoAnalysisManager$AnalysisWorkerCommand.applyTo(AutoAnalysisManager.java:1699)
	... 10 more
  (ConsoleErrorDisplay.java:45) 

[ghidra007]

@Wall-AF Thanks for the stack trace. I can add better checking for that case so it doesn't blow up. I've been told that the other ask for checks for non-standard thiscalls is a very big and complicated ask in general so not something this script would be able to easily do either. In any case, the other ticket you linked can be used to address that issue. This ticket will address the stack trace.