Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free-form C structure entry mechanism #4327

Open
Swyter opened this issue Jun 8, 2022 · 10 comments
Open

Free-form C structure entry mechanism #4327

Swyter opened this issue Jun 8, 2022 · 10 comments
Assignees
Labels
Status: Future This has future value but is not being worked on at this time

Comments

@Swyter
Copy link

Swyter commented Jun 8, 2022

Before closing #4229 there was a small discussion about adding a C-struct text mode editor that would make it easier to copy-paste and duplicate chunks between elements and in and out of the program.

There @ghidra1 said that the team had brainstormed about it in the past.

Personally, I think the all-or-nothing way one has to import and export all their existing headers/types in one go can be severely improved or complemented. When importing you need to do a lot of prepping and dial-tuning, checking that it isn't in read-write mode, and even if it succeeds it replaces the whole data block so you essentially need to move the imported data somewhere after the import and use a dummy/transient/disposable one for staging (in the likely case you are doing it in pieces).

Sometimes I change the enum or struct externally and it would be painless to do a direct copy-paste. Or the other way around, exporting small pieces of fields, or cloning them between data types, as well as quickly refactor in Ghidra by moving them around. It would make things more interoperable.

There is already something very similar for function signatures, and it works very well, so I feel like this would improve everyone's workflow.

Let me know what you think, and thanks for reading.

@ghidra1
Copy link
Collaborator

ghidra1 commented Jun 9, 2022

There is already something very similar for function signatures, and it works very well, so I feel like this would improve everyone's workflow.

If you are referring to the Function Editor's parser this is know to have many issues and does not leverage the C Parser.

@ghidra1
Copy link
Collaborator

ghidra1 commented Jun 9, 2022

When importing you need to do a lot of prepping and dial-tuning, checking that it isn't in read-write mode, and even if it succeeds it replaces the whole data block so you essentially need to move the imported data somewhere after the import and use a dummy/transient/disposable one for staging (in the likely case you are doing it in pieces).

I am completely confused by this statement and uncertain how this applies to using the C Parser - assuming that is what you were referring to.

@ghidra1
Copy link
Collaborator

ghidra1 commented Jun 9, 2022

One of the biggest obstacles with using the C Parser for re-parsing Ghidra datatypes is the fact that our type naming is not C-compliant and in many cases can drag in namespace and template constructs that the C Parser can not handle. In addition, the C Parser can not be used for manipulating non-packed Ghidra structures which are frequently incomplete in their definition and precise with component placement (i.e., component offsets).

@Swyter
Copy link
Author

Swyter commented Jun 9, 2022

I am completely confused by this statement and uncertain how this applies to using the C Parser - assuming that is what you were referring to.

I'm talking about Ghidra data type archives, and how you need to manually toggle the read-only mode back and forth (the File is in use error after configuring everything), and how a successful import seems to overwrite the whole archive. Which if you are incrementally updating or appending a single data type in a new header forces you to make temporary files to copy them over.

That's how I have been using it so far, at least.

If you are referring to the Function Editor's parser this is know to have many issues and does not leverage the C Parser.

I don't know the underlying system, it may be different and using an incomplete parser with many holes. But it certainly saves a lot of time. One can always revert on weird results.

Even if it starts small as a way of importing and exporting enums and maybe just importing simple structs without roundtrip it would come in handy. As you said, no matter what there's currently no good way of bridging this functionality.

To give some examples of workflow, I use 010 Editor all the time to reverse file format structs and apply binary templates. Even if I need to manually tweak padding/dummy bytes afterwards it would be a big improvement whenever I need to share common data type stuff. Having an immediate skeleton on Ctrl+C, Ctrl+V.

@ryanmkurtz ryanmkurtz added the Status: Triage Information is being gathered label Jun 9, 2022
@Swyter
Copy link
Author

Swyter commented Jun 10, 2022

After reading the manual again (it's been a while) seems like at least the Preview Window in the Data Type Manager can be used to quickly copy parts of enums using a C-ish syntax, and the values are neater and even hex-encoded, unlike when exporting as C-header. No commas, though.

Having 4000 constants in there with the preview pane opened brings the whole program to a crawl, but that's a different issue. If you want some example, try importing this in Ghidra and see how it fares every time that pane gets refreshed.

@Swyter
Copy link
Author

Swyter commented Jun 10, 2022

Another seemingly buggy feature is the fact that after completing Parse to File... for some already-loaded GDT one seemingly needs to essentially reopen or reload said file for the updated contents to show. This is something that threw me off the first time around, thinking that it just wasn't working, but just using Open for Editing seems to do the trick to refresh it.

I would definitely like a warning about potentially losing any preexisting GDT contents after using Parse to File.... I don't think that's clear enough when it warns about overwriting it. I initially assumed it would merge the types and categories already there.

@Swyter
Copy link
Author

Swyter commented Jun 10, 2022

I don't know why when asked about overwriting GDTs it can't figure out that the lock is local and owned by the current tool instance and proceed anyway. Seems more user friendly, it's going to essentially rewrite it anyway. Getting stuck for a while and then throwing some Archive Overwrite Failed, forcing me to manually click on Close for Editing and retry isn't fun.

@Swyter
Copy link
Author

Swyter commented Jun 10, 2022

And this is just cosmetic, but when pasting directly a quoted path like "C:\Users\ghidrafan69\folder\github\gforce-tools\gforce_hashcodes.enum.h" in the Choose Source Files dialog it seems to only strip the initial "s; leaving the trailing ones there. For example, this is what happens when you start to add extra trailing quotes.

image image

The first time I used it and it didn't appear to work (due to having-to-re-open-the-GDT-to-refresh thing above) this looked pretty suspect. But parsing the files you see in the image actually works.

@ghidra1 ghidra1 assigned emteere and unassigned ghidra1 Jun 10, 2022
@emteere emteere added Status: Prioritize This is currently being prioritized and removed Status: Triage Information is being gathered labels Jul 13, 2022
@emteere emteere added Status: Internal This is being tracked internally by the Ghidra team and removed Status: Prioritize This is currently being prioritized labels Jul 28, 2022
@emteere
Copy link
Contributor

emteere commented Jul 28, 2022

We've had this on the queue for a while adding it to our GP tickets to do soon.

@mumbel
Copy link
Contributor

mumbel commented Jul 28, 2022

@emteere Do you foresee this being incorporated into structure merges (update/commit to archive and in VT) or just creation/editing? Hopefully somewhat the same logic and its just a matter of squeezing into the UI once the hard part is done.

@ghidra1 ghidra1 added Status: Future This has future value but is not being worked on at this time and removed Status: Internal This is being tracked internally by the Ghidra team labels Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Future This has future value but is not being worked on at this time
Projects
None yet
Development

No branches or pull requests

5 participants