fix(ghidra): clear wrong no-return flags before serving a session (#1419)

* fix(ghidra): clear wrong no-return flags before serving a session

Auto-analysis can flag ordinary functions as no-return (stream readers,
__cxa_guard_acquire, ...). Their callers then end at the call and the
decompiler truncates them. Run a postScript before the bridge that clears the
flag on functions containing a real RET and re-disassembles the call sites that
were terminators.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

* fix(ghidra): refit in place and scope no-return call-site repair

Keep function types by refitting the owning functions with fixupFunctionBody
instead of removing and recreating them. Only redo call sites whose callee was
actually cleared (including thunks to it), skip sites followed by a function
start or x86 padding so real end calls do not run on, and always log the result.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>

* fix(ghidra): constrain no-return repair to admitted return evidence

---------

Co-authored-by: Claude Sonnet 5.5 <[email protected]>
Co-authored-by: morluto <[email protected]>

bridge/ghidra/ReaGhidraNoReturnFix.java

@@ -0,0 +1,89 @@
+import ghidra.app.cmd.function.CreateFunctionCmd;
+import ghidra.app.script.GhidraScript;
+import ghidra.program.model.address.Address;
+import ghidra.program.model.listing.*;
+import ghidra.program.model.symbol.FlowType;
+import ghidra.program.model.pcode.PcodeOp;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
+
+/**
+ * Undo wrong no-return flags from auto-analysis. A function flagged no-return that contains a
+ * real RET is an ordinary function (stream readers, cxa_guard_acquire, ...). Callers treated the
+ * call as a terminator and the decompiler truncated them. Clear the flag, then re-disassemble only
+ * the call sites of the functions that were cleared and refit their owning functions in place.
+ */
+public final class ReaGhidraNoReturnFix extends GhidraScript {
+  @Override
+  public void run() throws Exception {
+    // Windows P0 does not admit analysis-database mutation.
+    if (System.getProperty("os.name", "").startsWith("Windows")) return;
+    FunctionManager fm = currentProgram.getFunctionManager();
+    Listing li = currentProgram.getListing();
+    Set<Address> cleared = new HashSet<>();
+    for (Function f : fm.getFunctions(true)) {
+      monitor.checkCancelled();
+      if (!f.hasNoReturn() || f.isThunk()) continue;
+      for (Instruction ins : li.getInstructions(f.getBody(), true)) {
+        FlowType ft = ins.getFlowType();
+        boolean decodedReturn = false;
+        for (PcodeOp operation : ins.getPcode()) {
+          if (operation.getOpcode() == PcodeOp.RETURN) decodedReturn = true;
+        }
+        if (decodedReturn && !ft.isJump() && !ft.isCall()) {
+          f.setNoReturn(false);
+          cleared.add(f.getEntryPoint());
+          break;
+        }
+      }
+    }
+    // A thunk inherits the flag of the function it forwards to.
+    for (Function f : fm.getFunctions(true)) {
+      monitor.checkCancelled();
+      if (!f.isThunk()) continue;
+      Function target = f.getThunkedFunction(true);
+      if (target != null && cleared.contains(target.getEntryPoint())) {
+        cleared.add(f.getEntryPoint());
+      }
+    }
+    int redone = 0;
+    TreeSet<Address> owners = new TreeSet<>();
+    if (!cleared.isEmpty()) {
+      List<Address> sites = new ArrayList<>();
+      for (Instruction ins : li.getInstructions(true)) {
+        monitor.checkCancelled();
+        FlowType ft = ins.getFlowType();
+        if (!(ft.isCall() && ft.isTerminal())) continue;
+        Address[] flows = ins.getFlows();
+        if (flows.length == 0 || !cleared.contains(flows[0])) continue;
+        // Do not redisassemble into an independently identified function.
+        Address next = ins.getMaxAddress().add(1);
+        if (fm.getFunctionAt(next) != null || !currentProgram.getMemory().contains(next)) continue;
+        sites.add(ins.getAddress());
+      }
+      for (Address s : sites) {
+        monitor.checkCancelled();
+        Instruction ins = li.getInstructionAt(s);
+        if (ins == null) continue;
+        Function owner = fm.getFunctionContaining(s);
+        if (owner != null && !owner.isThunk()) owners.add(owner.getEntryPoint());
+        clearListing(ins.getMinAddress(), ins.getMaxAddress());
+        disassemble(s);
+        redone++;
+      }
+      for (Address e : owners) {
+        monitor.checkCancelled();
+        Function f = fm.getFunctionAt(e);
+        // Refit the body in place so the name, signature and calling convention survive.
+        if (f != null) CreateFunctionCmd.fixupFunctionBody(currentProgram, f, monitor);
+      }
+      // No analyzeChanges here: on libpl2.so it adds about 70 s, which pushes startup past the 330 s deadline.
+    }
+    println(
+        "REA no-return fix: cleared=" + cleared.size() + " sites=" + redone + " refit=" + owners.size());
+  }
+
+}

package.json

@@ -50,6 +50,7 @@
     "bridge/ghidra/ReaGhidraBridge.java",
     "bridge/ghidra/ReaGhidraExtensions.java",
     "bridge/ghidra/ReaGhidraPrepareCom.java",
+    "bridge/ghidra/ReaGhidraNoReturnFix.java",
     "bridge/android/ReaJadxBridge.java",
     "bridge/mitmproxy/capture.py",
     "scripts/rea.mjs",

scripts/verify-packaged-ghidra-windows.mjs

@@ -172,7 +172,11 @@ const digest = (bytes) => createHash("sha256").update(bytes).digest("hex");
 const runtimeParent = join(workspace, "runtime with spaces");
 await mkdir(runtimeParent);
 const callerDirectory = join(workspace, "caller cwd with spaces");
-const scriptCollisions = ["ReaGhidraBridge.java", "ReaGhidraPrepareCom.java"];
+const scriptCollisions = [
+  "ReaGhidraBridge.java",
+  "ReaGhidraPrepareCom.java",
+  "ReaGhidraNoReturnFix.java",
+];
 await mkdir(callerDirectory);
 for (const name of scriptCollisions) await mkdir(join(callerDirectory, name));
 const environment = {

src/ghidra/GhidraLauncher.ts

@@ -134,6 +134,7 @@ export class GhidraHeadlessLauncher implements GhidraLauncher {
       if (isAborted(options.signal))
         return err(new AnalysisCancelledError("open_binary"));
       const headlessArguments = ghidraHeadlessArguments({
+        platform,
         projectRoot: paths.projectRoot,
         targetPath: session.targetPath,
         bridgeScriptPath: this.options.bridgeScriptPath,
@@ -358,6 +359,7 @@ const cleanupStartedProcess = (
 
 /** Paths encoded into one bounded analyzeHeadless invocation. */
 export interface GhidraHeadlessArgumentOptions {
+  readonly platform?: NodeJS.Platform;
   readonly projectRoot: string;
   readonly targetPath: string;
   readonly bridgeScriptPath: string;
@@ -411,6 +413,12 @@ export const ghidraHeadlessArguments = (
         join(dirname(options.bridgeScriptPath), "ReaGhidraPrepareCom.java"),
       ]
     : []),
+  ...((options.platform ?? process.platform) === "win32"
+    ? []
+    : [
+        "-postScript",
+        join(dirname(options.bridgeScriptPath), "ReaGhidraNoReturnFix.java"),
+      ]),
   "-postScript",
   // Ghidra checks the caller's cwd before scriptPath for a basename. Select
   // the packaged source explicitly so unrelated entries cannot shadow it.

tests/boundary/providers/ghidra/ghidraLauncher.test.ts

@@ -134,6 +134,7 @@ describe("Ghidra headless launcher", () => {
   });
   it("builds a read-only ephemeral import with its bridge descriptor", () => {
     const arguments_ = ghidraHeadlessArguments({
+      platform: "darwin",
       projectRoot: "/tmp/project",
       targetPath: "/tmp/target",
       bridgeScriptPath: "/package/bridge/ReaGhidraBridge.java",
@@ -147,14 +148,32 @@ describe("Ghidra headless launcher", () => {
       ["-log", "/tmp/ghidra.log"],
       ["-scriptlog", "/tmp/script.log"],
       ["-scriptPath", "/package/bridge"],
-      ["-postScript", "/package/bridge/ReaGhidraBridge.java"],
+      ["-postScript", "/package/bridge/ReaGhidraNoReturnFix.java"],
     ]);
-    expect(arguments_[arguments_.indexOf("-postScript") + 2]).toBe(
-      "/tmp/session.json",
-    );
+    // The no-return repair must finish before the bridge starts serving.
+    const fix = arguments_.indexOf("/package/bridge/ReaGhidraNoReturnFix.java");
+    const bridge = arguments_.indexOf("/package/bridge/ReaGhidraBridge.java");
+    expect(fix).toBeLessThan(bridge);
+    expect(arguments_[bridge + 1]).toBe("/tmp/session.json");
     expect(arguments_).toContain("-readOnly");
     expect(arguments_).toContain("-deleteProject");
   });
+});
+
+describe("Ghidra Windows launcher", () => {
+  it("keeps analysis repair outside Windows P0 authority", () => {
+    const args = ghidraHeadlessArguments({
+      platform: "win32",
+      projectRoot: "/tmp/project",
+      targetPath: "/tmp/target",
+      bridgeScriptPath: "/package/bridge/ReaGhidraBridge.java",
+      descriptorPath: "/tmp/session.json",
+      ghidraLogPath: "/tmp/ghidra.log",
+      scriptLogPath: "/tmp/script.log",
+    });
+    expect(args).not.toContain("/package/bridge/ReaGhidraNoReturnFix.java");
+    expect(args).toContain("/package/bridge/ReaGhidraBridge.java");
+  });
 
   it("wraps the Windows batch launcher without enabling a Node shell", () => {
     const command = ghidraHeadlessCommand({