ghidra icon indicating copy to clipboard operation
ghidra copied to clipboard

Typedef Editor

Open astrelsky opened this issue 3 years ago • 8 comments

Is your feature request related to a problem? Please describe. I'm always frustrated when I need to change the base DataType for a TypeDef and can't readily do so.

Describe the solution you'd like An editor window to edit the TypeDef like there is for composites and function signatures.

Describe alternatives you've considered Opening the python interpreter, creating a new TypeDefDataType to the new base type and using DataType.replaceWith. This gets old pretty quick.

astrelsky avatar Aug 22 '22 11:08 astrelsky

Relates to #3113

dragonmacher avatar Aug 22 '22 16:08 dragonmacher

Modification of a typedef's base type is not currently permitted at the API level. Allowing a datatype to change its size carries with it the risk that any uses of it may become broken and no longer fit within their varnode container (e.g., variables, memory data). With the potential for widespread use of a single Typedef, the negative consequences of a typedef size change could be huge. This is not to say a datatype-replacement is any safer but is allowed. A single composite size change generally has a smaller impact over a primitive size change that could be associated a typedef.

Done properly, this capability will require a Typedef API change and impact the TypedefDB implementation.

A Typedef editor should warn user if the base datatype size changes and restrict disallowed changes (TBD, e.g., pointer typedefs, use of dynamic base datatypes, etc.).

ghidra1 avatar Aug 23 '22 01:08 ghidra1

99% of the time I change a typedef is to fix the wrong size.

What does DataType.replaceWith do if the contents of a TypeDef cannot be changed? Is it that the TypeDef api doesn't explicitly support it and replaceWith is some sort of loophole?

astrelsky avatar Aug 23 '22 09:08 astrelsky

I stand corrected. I did not realize DataType.replaceWith was on the public interface - Ugh. I thought you were using DataTypeManager.replaceDataType. There is quite a bit on the public DataType interface which should not be (e.g., change callbacks) - this may be one of them. It's on my list to eliminate some of these at some point. It is too easy to do something really bad for which there are no safeguards (e.g, replace integer typedef with dynamic typedef such as PngDataType). If Typedef was intended to support this it should have a method Typedef.setBaseDataType - which is what I was considering above. The replaceWith has the benefit of pulling in the settings which would be important for a Pointer Typedef - although doing such a replacement after the type has been used has its own issues (e.g., prior analysis and markup based on the old typedef will be wrong). There is the potential for a bunch of stale markup which will not get corrected automatically.

ghidra1 avatar Aug 24 '22 14:08 ghidra1

I see. I will definitely be more careful about using it. However, does the same not apply when editing a Composite which has been applied to the listing? It's size doesn't even need to change and you can see the markup is stale just by converting one of its components from an integer to a pointer.

I like doing bad things. Bad things you shouldn't be able to do are my favorite.

astrelsky avatar Aug 24 '22 17:08 astrelsky

I like doing bad things. Bad things you shouldn't be able to do are my favorite.

I like to turn off my computer by holding in the power button. It's a real adrenaline rush.

ryanmkurtz avatar Aug 24 '22 18:08 ryanmkurtz

I like doing bad things. Bad things you shouldn't be able to do are my favorite.

I like to turn off my computer by holding in the power button. It's a real adrenaline rush.

You should try a hammer. It shuts down faster with just the right amount of force.

Also, in my experience, turning it off by holding the power button works best while updating. Especially during a bios update. Provides an even bigger adrenaline rush when it's not your computer.

astrelsky avatar Aug 24 '22 18:08 astrelsky

I see. I will definitely be more careful about using it. However, does the same not apply when editing a Composite which has been applied to the listing? It's size doesn't even need to change and you can see the markup is stale just by converting one of its components from an integer to a pointer.

Yes, there can be fallout with composite changes but generally does not impact variable variable storage allocations. That and there are no issues with it changing its stripes as could be done with a Typedef. So its more a matter of how much badness could occur. Typedefs will likey impact many more storage assignments than a structure.

ghidra1 avatar Aug 25 '22 14:08 ghidra1

@astrelsky Attached is a Ghidra Script ReplaceTypedefScript which will interactively facilitate replacing an existing Typedef selected in the Data Type Tree with a new one using a selected base datatype. NOTE: error handling could be improved somewhat and settings are not set on new typedef. Would this satisfy your need? At this point I am reluctant to make Typedef mutable. ReplaceTypedefScript.txt

ghidra1 avatar Oct 13 '22 17:10 ghidra1

@ghidra1 if I understand correctly, DataTypeManager.replaceDataType will iterate over all datatypes, find which ones contain said typedef and replace the usage of the old typedef with the new one? I most certainly don't want to iterate over a few million types which are using a poorly typed uint64_t that ended up 4 bytes and replace each uint64_t with a new one. It is much better to correct the existing uint64_t and make only one change.

I really do hate to be like this, and I would never share it, but if the replaceWith method is removed my next approach would be to write a script to alter the DatabaseObject directly and swap the underlying type. (Don't try that at home kids, I'm a professional. A professional idiot maybe, but a professional nonetheless)

The case with poorly typed fixed size integers is really the only consistent use case for modifying a typedef. That being said it does make sense for them to be immutable; it's just really painful for the edge cases where you need it to be mutable.

astrelsky avatar Oct 13 '22 18:10 astrelsky

The DataTypeManager knows what types make use of a Typedef (e.g., parent list). Due to the potential ripple affects of a change (e.g., size/packing/alignment), the replacment approach or a fully integrated approach is required. Slipping in a modified base datatype is inadequate on its own. The ripple affects can cause other structures to change there size/packing/alignment and the dominos continue to fall. DataTypeManager.replaceDataType for direct use by Data and Variables is a brute force search which is unfortunate (no index). While a properly integrated approach is feasible it would entail a larger effort at a low priority.

ghidra1 avatar Oct 13 '22 18:10 ghidra1

Considering the concerns with altering a typedef and the limited number of cases where it would be necessary to actually alter it instead of replacing it, I think that an editorial for this is not worth the effort to do properly.

astrelsky avatar Oct 14 '22 08:10 astrelsky