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

Add headless Ghidra tests to CI #9

Merged
merged 30 commits into from
Jan 27, 2024
Merged

Add headless Ghidra tests to CI #9

merged 30 commits into from
Jan 27, 2024

Conversation

mahaloz
Copy link
Member

@mahaloz mahaloz commented Nov 29, 2023

No description provided.

@mahaloz mahaloz marked this pull request as draft November 29, 2023 22:39
@mahaloz
Copy link
Member Author

mahaloz commented Dec 6, 2023

oofers bad rebase

@mahaloz mahaloz force-pushed the feat/headless_and_ci branch from 316568c to d39862a Compare December 6, 2023 23:03
@Flipout50 Flipout50 force-pushed the feat/headless_and_ci branch from d39862a to 316568c Compare December 6, 2023 23:04
@mahaloz mahaloz force-pushed the feat/headless_and_ci branch from 316568c to 45fb901 Compare December 6, 2023 23:07
@mahaloz mahaloz changed the title Add headless CI skeleton to workflow Add headless Ghidra tests to CI Dec 21, 2023
@mahaloz
Copy link
Member Author

mahaloz commented Dec 21, 2023

@Flipout50 I looked at the failure log for the CI, are you sure GhidraServer is actually running? My hunch is that its crashing on run and not actually starting up. The way I would test this is the get the proc after using Popen, then print p.poll() and whatever is on stdout and stderr. Then commit that and let the CI run. Check the log, the prints should be there. I know debugging GitHub is kinda aids... if you find a better way to debug lmk :)

@mahaloz
Copy link
Member Author

mahaloz commented Jan 8, 2024

@Flipout50 don't forget to rebase!

@Flipout50 Flipout50 force-pushed the feat/headless_and_ci branch from 7f5b29b to 4fa0ce4 Compare January 9, 2024 01:22
@mahaloz
Copy link
Member Author

mahaloz commented Jan 9, 2024

Ayo @Flipout50, the CI ran for 5 hours on commit a7797ce until I canceled it. Maybe you forgot to shutdown the Ghidra side server in the code?

tests/tests.py Show resolved Hide resolved
@@ -46,6 +51,30 @@ def __init__(self, loop_on_plugin=True, **kwargs):

self.loop_on_plugin = loop_on_plugin

# Startup headless ghidra binary
if self.headless:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move any code you can into _init_headless_componenets

@Flipout50 Flipout50 force-pushed the feat/headless_and_ci branch from b60752a to 24b24b3 Compare January 16, 2024 23:04
@mahaloz
Copy link
Member Author

mahaloz commented Jan 23, 2024

@Flipout50 reminder: give an update when you can here and mark it ready for review when it's ready :).

@Flipout50
Copy link
Collaborator

Flipout50 commented Jan 24, 2024

@mahaloz I hit a big snag. I was planning to just add a simple test to rename a function to get this branch merged. Turns out thats broken too. There is a NoneType somehow making its way into ghidraBridge and I havn't been able to figure out how its getting there. For some reason _set_function_header is returning None even though all that is in the function is return False??? I found out through testing that the _set_function_header that I was looking at isn't even getting called so im trying to hunt down where the code is going. It ends up trying to call getService() from ghidra bridge on NoneType which causes the transaction to fail.

@Flipout50
Copy link
Collaborator

Flipout50 commented Jan 25, 2024

UPDATE: I finally found where the bug is. Its when the function header gets changed, libbs calls typestr_to_gtype on the string "char". In this function on line 685 in interface.py, self.ghidra.getState().getTool() returns None, and we are attempting to call getService(dtm_service_class) on this NoneType. Should I open an issue?

@Flipout50 Flipout50 self-requested a review January 25, 2024 02:09
@Flipout50 Flipout50 marked this pull request as ready for review January 25, 2024 02:10
@mahaloz
Copy link
Member Author

mahaloz commented Jan 25, 2024 via email

@Flipout50
Copy link
Collaborator

NationalSecurityAgency/ghidra#934 Ill try some solutions here. Apparently you can't get the tool from headless

@mahaloz
Copy link
Member Author

mahaloz commented Jan 25, 2024 via email

@Flipout50 Flipout50 marked this pull request as draft January 25, 2024 03:26
@Flipout50 Flipout50 marked this pull request as ready for review January 25, 2024 03:26
@Flipout50 Flipout50 force-pushed the feat/headless_and_ci branch from 888e0a2 to d740eb9 Compare January 25, 2024 03:47
@Flipout50
Copy link
Collaborator

I see. All uses of getTool will need to be updated

On Wed, Jan 24, 2024 at 4:56 PM, Flipout50 @.(mailto:On Wed, Jan 24, 2024 at 4:56 PM, Flipout50 < wrote: [NationalSecurityAgency/ghidra#934](NationalSecurityAgency/ghidra#934) Ill try some solutions here. Apparently you can't get the tool from headless — Reply to this email directly, [view it on GitHub](#9 (comment)), or unsubscribe. You are receiving this because you were mentioned.Message ID: @.>

There is only one other use in interface.py but it occurs in a gui function. I believe this doesn't get called in headless mode, so we can leave it alone right?

@mahaloz
Copy link
Member Author

mahaloz commented Jan 26, 2024 via email

@mahaloz mahaloz merged commit 1f83025 into main Jan 27, 2024
1 check passed
@mahaloz mahaloz deleted the feat/headless_and_ci branch January 28, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants