Fix misleading errors of disassemble, il_function, FLIRT and decompile

* `disassemble` at an unmapped address said "no output for a command
  expecting JSON"; with command status it now fails with Rizin's "Failed
  to read chunk ... at 0xdeadbeef" (covered by a new test).
* `il_function`: `plf` fails without a message where there is no
  function, so say so when `afij` finds none; empty output of the text
  command is the new `EmptyOutput`, not `EmptyJson`.
* `flirt_apply` reported "Found 0 FLIRT signatures" for a missing file,
  since `Fs` returns OK and only logs "Can't open": check that a plain
  path exists, fail on logged errors, and return the number of matched
  functions.
* `flirt_apply_sigdb` checked its filter, a substring of the sigdb paths,
  with the path validator, rejecting `@` and other characters; quote it
  instead. It now returns the number of functions named (counted in the
  `flirt` flag space) and the signature files applied.
* `decompile` moved to the next engine on any error and ended with
  "install the plugin" even after a timeout, retrying a wedged session
  twice. Only a missing plugin (NONEXISTINGCMD) or an empty result moves
  on now; any other failure is returned as is. The skipped engines and why
  are in the structured result.
* A pipe to Rizin before 0.10 sees a failure only as no output plus an
  error message: `il_function` checks for a function on empty output as
  well, and `decompile` treats an error message without output as a
  failure of that engine.

README.md

@@ -144,7 +144,9 @@ produces the full listing, which the server then slices.
 - **FLIRT.** The `flirt_*` tools wrap Rizin's built-in `F` command family:
   `flirt_list`/`flirt_apply_sigdb` work against the signature database (sigdb)
   shipped with Rizin, while `flirt_apply`/`flirt_dump`/`flirt_create` work with
-  individual `.sig`/`.pat` files. (`function_signature_bytes` is unrelated to
+  individual `.sig`/`.pat` files. The two apply tools return how many functions
+  the signatures named; a missing file is an error, even though Rizin's `Fs`
+  itself reports success for it. (`function_signature_bytes` is unrelated to
   FLIRT — it just returns a function's masked byte pattern.)
 - **Silhouette.** `silhouette`/`silhouette_test` drive the
   [rz-silhouette](https://github.com/rizinorg/rz-silhouette) plugin's `sil`

src/error.rs

@@ -86,6 +86,13 @@ pub enum RizinError {
         detail: String,
     },
 
+    /// A text command succeeded but printed nothing.
+    #[error("rizin printed nothing for: {cmd}")]
+    EmptyOutput {
+        /// The command.
+        cmd: String,
+    },
+
     /// Rizin ran the command and reported that it failed.
     #[error("rizin command failed ({status}): {cmd}{}", fmt_detail(.detail))]
     CommandFailed {
@@ -175,6 +182,7 @@ impl RizinError {
             | InvalidArgument { .. }
             | IllegalCommand { .. }
             | EmptyJson { .. }
+            | EmptyOutput { .. }
             | FileNotFound(_)
             | OpenFailed { .. }
             | NotPermitted(_) => ErrorData::invalid_params(self.to_string(), None),

src/params.rs

@@ -341,7 +341,9 @@ pub struct FlirtFileParams {
 /// Parameters for `flirt_apply_sigdb`.
 #[derive(Debug, Default, Deserialize, JsonSchema)]
 pub struct FlirtSigdbParams {
-    /// Optional filter to select a subset of signatures from the sigdb.
+    /// Optional filter selecting the signature files whose sigdb path contains
+    /// it (e.g. `elf/x86/64` or `libc`); without one, the files matching the
+    /// binary's format, architecture, and bits are applied.
     #[serde(default)]
     pub filter: Option<String>,
 }

src/server.rs

@@ -307,6 +307,27 @@ impl RizinServer {
         Ok(())
     }
 
+    /// Whether a function covers the location of `suffix` (an `at_suffix`).
+    async fn has_function(&self, suffix: &str) -> bool {
+        self.manager
+            .cmdj(&format!("afij{suffix}"))
+            .await
+            .is_ok_and(|v| v.as_array().is_some_and(|a| !a.is_empty()))
+    }
+
+    /// The number of flags in the `flirt` flag space, which FLIRT matches
+    /// create.
+    async fn flirt_flag_count(&self) -> Result<u64, ErrorData> {
+        let spaces = self.cmdj_optional("fslj", json!([])).await?;
+        Ok(spaces
+            .as_array()
+            .into_iter()
+            .flatten()
+            .find(|s| s.get("name").and_then(Value::as_str) == Some("flirt"))
+            .and_then(|s| s.get("count").and_then(Value::as_u64))
+            .unwrap_or(0))
+    }
+
     /// Map a missing plugin command to a hint naming the plugin.
     fn plugin_hint(e: RizinError, hint: &str) -> ErrorData {
         match e {
@@ -754,22 +775,47 @@ impl RizinServer {
             }
         };
 
+        // Only a missing plugin (or an empty result) moves on to the next
+        // engine: an engine that ran and failed (e.g. no function there) would
+        // fail the same way in the next one, and a timeout or dead session
+        // must not be retried.
+        let mut skipped = serde_json::Map::new();
         for (engine, cmd) in &candidates {
-            match self.manager.cmd(cmd).await {
-                Ok(out) if !out.trim().is_empty() => {
-                    return Ok(self.text_result_meta(out, json!({ "decompiler": engine })));
+            let out = self.manager.exec(cmd).await.mcp()?;
+            match crate::backend::check(cmd, out) {
+                Ok(out) if !out.output.trim().is_empty() => {
+                    return Ok(self.text_result_meta(
+                        out.output,
+                        json!({ "decompiler": engine, "skipped": skipped }),
+                    ));
                 }
-                Ok(_) => {} // empty: plugin missing or nothing to show, try next
-                Err(e @ (RizinError::NoFileOpen | RizinError::SessionClosed)) => {
-                    return Err(e.to_error_data());
+                // A pipe to Rizin before 0.10 sees a failure only as no output
+                // and an error message.
+                Ok(out) if out.messages.iter().any(|m| m.starts_with("ERROR")) => {
+                    return Err(RizinError::CommandFailed {
+                        cmd: cmd.clone(),
+                        status: out.status,
+                        detail: out.detail(),
+                    }
+                    .to_error_data());
                 }
-                Err(_) => {}
+                Ok(_) => {
+                    skipped.insert(engine.to_string(), json!("no output"));
+                }
+                Err(RizinError::CommandFailed {
+                    status: CmdStatus::NonExistingCmd,
+                    ..
+                }) => {
+                    skipped.insert(engine.to_string(), json!("not installed"));
+                }
+                Err(e) => return Err(e.to_error_data()),
             }
         }
 
-        let hint = if which == "auto" || which == "ghidra" || which == "jsdec" {
-            "no decompiler output. Install the rz-ghidra (pdg) or jsdec (pdd) plugin, or use \
-             decompiler=\"pseudo\"."
+        let missing = skipped.values().any(|v| v == "not installed");
+        let hint = if missing {
+            "no decompiler output: the requested engine is not installed. Install the \
+             rz-ghidra (pdg) or jsdec (pdd) plugin, or use decompiler=\"pseudo\"."
         } else {
             "no decompiler output for the function at this location."
         };
@@ -1135,40 +1181,76 @@ impl RizinServer {
     #[tool(
         description = "Apply FLIRT signatures from Rizin's signature database (sigdb) to the \
         loaded binary, naming matched library functions. An optional filter narrows which \
-        signatures are tried (Fa)."
+        signature files are tried. Returns the number of matched functions and the files \
+        applied (Fa)."
     )]
     async fn flirt_apply_sigdb(
         &self,
         Parameters(p): Parameters<FlirtSigdbParams>,
     ) -> Result<CallToolResult, ErrorData> {
+        // The filter is a substring of the signature files' sigdb paths
+        // (e.g. `elf/x86/64`), so it is quoted rather than path-checked.
         let cmd = match &p.filter {
-            Some(f) => format!("Fa {}", validate_path(f).mcp()?),
+            Some(f) => format!("Fa {}", quote_arg(f).mcp()?),
             None => "Fa".to_string(),
         };
+        let before = self.flirt_flag_count().await?;
         let out = self.manager.cmd(&cmd).await.mcp()?;
-        Ok(self.text_result(if out.trim().is_empty() {
-            "Applied sigdb signatures (no new matches reported).".to_string()
-        } else {
-            out
-        }))
+        let applied = self.flirt_flag_count().await?.saturating_sub(before);
+        let files: Vec<&str> = out
+            .lines()
+            .filter_map(|l| l.strip_prefix("Applying "))
+            .collect();
+        Ok(self.json_result(json!({
+            "matched_functions": applied,
+            "signature_files": files,
+        })))
     }
 
     /// Apply a FLIRT signature file (`Fs`).
     #[tool(
-        description = "Open a FLIRT signature file (.sig or .pat) and apply its signatures to \
-        the loaded binary, naming matched functions (Fs)."
+        description = "Open a FLIRT signature file (.sig or .pat; a glob is accepted) and apply \
+        its signatures to the loaded binary, naming matched functions. Returns the number of \
+        matched functions (Fs)."
     )]
     async fn flirt_apply(
         &self,
         Parameters(p): Parameters<FlirtFileParams>,
     ) -> Result<CallToolResult, ErrorData> {
         let path = validate_path(&p.path).mcp()?;
-        let out = self.manager.cmd(&format!("Fs {path}")).await.mcp()?;
-        Ok(self.text_result(if out.trim().is_empty() {
-            format!("Applied FLIRT signatures from {path}.")
-        } else {
-            out
-        }))
+        // `Fs` takes a glob; a plain path must exist.
+        if !path.contains(['*', '?', '[']) && !std::path::Path::new(&path).is_file() {
+            return Err(RizinError::FileNotFound(path).to_error_data());
+        }
+        let cmd = format!("Fs {path}");
+        let out = self.manager.exec(&cmd).await.mcp()?;
+        let out = crate::backend::check(&cmd, out).mcp()?;
+        // `Fs` reports success even when it cannot read a file, saying so only
+        // in an error message.
+        let errors: Vec<&str> = out
+            .messages
+            .iter()
+            .map(String::as_str)
+            .filter(|m| m.starts_with("ERROR"))
+            .collect();
+        if !errors.is_empty() {
+            return Err(RizinError::CommandFailed {
+                cmd,
+                status: CmdStatus::Error,
+                detail: errors.join("\n"),
+            }
+            .to_error_data());
+        }
+        let matched = out
+            .output
+            .split_whitespace()
+            .nth(1)
+            .and_then(|n| n.parse::<u64>().ok());
+        Ok(self.json_result(json!({
+            "path": path,
+            "matched_functions": matched,
+            "output": out.output.trim(),
+        })))
     }
 
     /// Dump a FLIRT signature file (`Fd`).
@@ -1274,13 +1356,30 @@ impl RizinServer {
         Parameters(p): Parameters<AtParams>,
     ) -> Result<CallToolResult, ErrorData> {
         let suffix = at_suffix(&p.at).mcp()?;
-        let out = self.manager.cmd(&format!("plf{suffix}")).await.mcp()?;
-        if out.trim().is_empty() {
-            return Err(RizinError::EmptyJson {
-                cmd: "plf".into(),
-                detail: String::new(),
+        let cmd = format!("plf{suffix}");
+        // `plf` fails without saying why (and before Rizin 0.10, a pipe only
+        // sees no output); the usual reason is that there is no function.
+        let no_function = || {
+            RizinError::InvalidArgument {
+                arg: p.at.clone().unwrap_or_else(|| "the current seek".into()),
+                reason: "no function there (analyze it first, e.g. with analyze_function)".into(),
             }
-            .to_error_data());
+            .to_error_data()
+        };
+        let out = match self.manager.cmd(&cmd).await {
+            Ok(out) => out,
+            Err(RizinError::CommandFailed { detail, .. })
+                if detail.is_empty() && !self.has_function(&suffix).await =>
+            {
+                return Err(no_function());
+            }
+            Err(e) => return Err(e.to_error_data()),
+        };
+        if out.trim().is_empty() {
+            if !self.has_function(&suffix).await {
+                return Err(no_function());
+            }
+            return Err(RizinError::EmptyOutput { cmd }.to_error_data());
         }
         Ok(self.text_result(out))
     }

tests/tools.rs

@@ -522,3 +522,83 @@ async fn disabled_tools_are_not_offered() {
     assert_eq!(err.code.0, -32602, "{err:?}");
     assert!(err.message.contains("not found"), "{err:?}");
 }
+
+#[tokio::test]
+async fn errors_say_what_went_wrong() {
+    let pat = std::env::temp_dir().join(format!("rizin-mcp-test-{}.pat", std::process::id()));
+    let pat = pat.to_string_lossy().into_owned();
+    for t in transports() {
+        let s = Session::start(t, |c| c.allow_write = true).await;
+        s.ok(
+            "open_file",
+            json!({ "path": target(), "analysis": "basic" }),
+        )
+        .await;
+
+        // An unmapped address fails with Rizin's reason, not just "no
+        // output" (which a pipe to Rizin before 0.10 still says, but with the
+        // reason attached).
+        let err = s
+            .call("disassemble", json!({ "at": "0xdeadbeef" }))
+            .await
+            .expect_err("unmapped");
+        assert!(matches!(err.code.0, -32001 | -32602), "{t:?}: {err:?}");
+        assert!(
+            err.message.contains("Failed to read") && err.message.contains("0xdeadbeef"),
+            "{t:?}: {}",
+            err.message
+        );
+
+        // `plf` fails silently where there is no function; the tool says so.
+        let err = s
+            .call("il_function", json!({ "at": "0x0" }))
+            .await
+            .expect_err("no function");
+        assert!(
+            err.message.contains("no function"),
+            "{t:?}: {}",
+            err.message
+        );
+
+        // A missing signature file is not "Found 0 FLIRT signatures".
+        let err = s
+            .call("flirt_apply", json!({ "path": "/nonexistent.sig" }))
+            .await
+            .expect_err("missing file");
+        assert!(
+            err.message.contains("file not found"),
+            "{t:?}: {}",
+            err.message
+        );
+
+        // Signatures made from this binary match it again.
+        s.ok("flirt_create", json!({ "path": pat })).await;
+        let applied = s.ok("flirt_apply", json!({ "path": pat })).await;
+        assert!(
+            applied["matched_functions"].as_u64().unwrap() > 0,
+            "{t:?}: {applied}"
+        );
+
+        // A sigdb filter may contain `@` and other shell characters.
+        let r = s
+            .ok("flirt_apply_sigdb", json!({ "filter": "x86@64;px" }))
+            .await;
+        assert_eq!(r["matched_functions"], 0, "{t:?}: {r}");
+
+        // A missing engine is skipped; one that fails is reported, not
+        // retried with the next engine under a misleading "install" hint.
+        let d = s
+            .call("decompile", json!({ "at": "main" }))
+            .await
+            .expect("decompile");
+        let meta = d.structured_content.unwrap();
+        assert_eq!(meta["decompiler"], "pseudo", "{t:?}: {meta}");
+        let err = s
+            .call("decompile", json!({ "at": "0x0", "decompiler": "pseudo" }))
+            .await
+            .expect_err("no function");
+        assert_eq!(err.code.0, -32001, "{t:?}: {err:?}");
+        assert!(err.message.contains("function"), "{t:?}: {}", err.message);
+    }
+    let _ = std::fs::remove_file(&pat);
+}