diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fb89732..cf76b45 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -123,8 +123,6 @@ jobs: if: ${{ !cancelled() }} runs-on: ${{ matrix.os }} timeout-minutes: 90 - # Windows Ghidra tests may exceed timeout on GH Actions free runners - continue-on-error: ${{ matrix.os == 'windows-latest' }} strategy: fail-fast: false matrix: @@ -196,7 +194,6 @@ jobs: if: ${{ !cancelled() }} runs-on: ${{ matrix.os }} timeout-minutes: 90 - continue-on-error: ${{ matrix.os == 'windows-latest' }} strategy: fail-fast: false matrix: @@ -268,7 +265,6 @@ jobs: if: ${{ !cancelled() }} runs-on: ${{ matrix.os }} timeout-minutes: 90 - continue-on-error: ${{ matrix.os == 'windows-latest' }} strategy: fail-fast: false matrix: diff --git a/src/ghidra/bridge.rs b/src/ghidra/bridge.rs index 0690cea..da71473 100644 --- a/src/ghidra/bridge.rs +++ b/src/ghidra/bridge.rs @@ -405,6 +405,32 @@ pub fn stop_bridge(project_path: &Path) -> Result<()> { .args(["/PID", &pid.to_string(), "/F", "/T"]) .output(); } + + // Wait for the process to actually die after SIGTERM/taskkill. + // Without this, the JVM may still hold the project lock when the + // next bridge tries to start (causes intermittent CI failures). + for _ in 0..100 { + if !is_pid_alive(pid) { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + + // Last resort: SIGKILL if SIGTERM wasn't enough + #[cfg(unix)] + if is_pid_alive(pid) { + warn!("SIGKILL bridge process {} (SIGTERM didn't work)", pid); + unsafe { + libc::kill(pid as i32, libc::SIGKILL); + } + // Brief wait for SIGKILL to take effect + for _ in 0..20 { + if !is_pid_alive(pid) { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + } } } diff --git a/src/ghidra/scripts/GhidraCliBridge.java b/src/ghidra/scripts/GhidraCliBridge.java index a32de24..0fd2883 100644 --- a/src/ghidra/scripts/GhidraCliBridge.java +++ b/src/ghidra/scripts/GhidraCliBridge.java @@ -1113,6 +1113,9 @@ public class GhidraCliBridge extends GhidraScript { try { JsonArray results = new JsonArray(); + + // Phase 1: Search pre-analyzed string data types from the listing. + // This is fast and returns strings Ghidra's analyzer has already classified. Listing listing = currentProgram.getListing(); DataIterator dataIter = listing.getDefinedData(true); @@ -1132,6 +1135,32 @@ public class GhidraCliBridge extends GhidraScript { } } + // Phase 2: If listing search found nothing and we have a pattern, + // fall back to raw memory scanning. This catches strings that Ghidra's + // analyzer didn't classify as string data types (common on PE binaries). + if (results.size() == 0 && !pattern.isEmpty()) { + Memory memory = currentProgram.getMemory(); + byte[] searchBytes = pattern.getBytes(java.nio.charset.StandardCharsets.UTF_8); + + Address addr = memory.getMinAddress(); + while (addr != null && results.size() < 100) { + Address found = memory.findBytes(addr, searchBytes, null, true, monitor); + if (found == null) break; + + // Try to extract the full null-terminated string at this address + String extracted = extractStringAt(memory, found, 4096); + if (extracted != null && !extracted.isEmpty()) { + JsonObject item = new JsonObject(); + item.addProperty("address", found.toString()); + item.addProperty("value", extracted); + item.addProperty("length", extracted.length()); + results.add(item); + } + + addr = found.add(Math.max(1, extracted != null ? extracted.length() : 1)); + } + } + JsonObject result = new JsonObject(); result.add("results", results); result.addProperty("count", results.size()); @@ -1141,6 +1170,25 @@ public class GhidraCliBridge extends GhidraScript { } } + /** + * Extract a printable string starting at the given address. + * Reads until a null byte, non-printable character, or maxLen is reached. + */ + private String extractStringAt(Memory memory, Address addr, int maxLen) { + try { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < maxLen; i++) { + byte b = memory.getByte(addr.add(i)); + if (b == 0) break; + if (b < 0x20 || b > 0x7e) break; // non-printable ASCII + sb.append((char) b); + } + return sb.length() > 0 ? sb.toString() : null; + } catch (Exception e) { + return null; + } + } + private JsonObject handleFindBytes(JsonObject args) { if (currentProgram == null) return errorResult("No program loaded"); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index cafb826..2c02cfa 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -133,6 +133,7 @@ pub fn ensure_test_project(project: &str, program: &str) { /// Tests connect to it via TCP using BridgeClient. pub struct DaemonTestHarness { port: u16, + pid: Option, data_dir: PathBuf, project: String, project_path: PathBuf, @@ -166,8 +167,14 @@ impl DaemonTestHarness { // Read port from port file let port = Self::wait_for_port(&project_path, Duration::from_secs(120))?; + // Store PID now so Drop can wait for it even if restart deletes the PID file + let pid = ghidra_cli::ghidra::bridge::read_pid_file(&project_path) + .ok() + .flatten(); + Ok(Self { port, + pid, data_dir, project: project.to_string(), project_path, @@ -234,25 +241,35 @@ impl DaemonTestHarness { impl Drop for DaemonTestHarness { fn drop(&mut self) { - // Read PID BEFORE shutdown (Java deletes PID file during shutdown) - let pid = ghidra_cli::ghidra::bridge::read_pid_file(&self.project_path) + // Read current PID from file (may differ from self.pid if restart changed it) + let file_pid = ghidra_cli::ghidra::bridge::read_pid_file(&self.project_path) .ok() .flatten(); // Use stop_bridge for proper graceful shutdown + force-kill let _ = ghidra_cli::ghidra::bridge::stop_bridge(&self.project_path); - // Wait for process to fully exit and release project lock. - // Critical on Windows where JVM cleanup is slow. - if let Some(pid) = pid { - let max_wait = if cfg!(windows) { - Duration::from_secs(30) - } else { - Duration::from_secs(10) - }; + // Collect all PIDs we need to wait for (original + current, deduplicated) + let mut pids_to_wait: Vec = Vec::new(); + if let Some(pid) = file_pid { + pids_to_wait.push(pid); + } + if let Some(pid) = self.pid { + if !pids_to_wait.contains(&pid) { + pids_to_wait.push(pid); + } + } + + // Wait for ALL known processes to fully exit and release project lock. + let max_wait = if cfg!(windows) { + Duration::from_secs(30) + } else { + Duration::from_secs(15) + }; + for pid in &pids_to_wait { let start = std::time::Instant::now(); while start.elapsed() < max_wait { - if !ghidra_cli::ghidra::bridge::is_pid_alive(pid) { + if !ghidra_cli::ghidra::bridge::is_pid_alive(*pid) { break; } std::thread::sleep(Duration::from_millis(500)); diff --git a/tests/readonly_tests.rs b/tests/readonly_tests.rs index 5e0839c..c0a285a 100644 --- a/tests/readonly_tests.rs +++ b/tests/readonly_tests.rs @@ -1267,7 +1267,9 @@ fn test_batch_invalid_file() { result.assert_failure(); assert!( - result.stderr.contains("not found") || result.stderr.contains("No such file"), + result.stderr.contains("not found") + || result.stderr.contains("No such file") + || result.stderr.contains("cannot find"), "Should contain file-not-found error. Got: {}", result.stderr );