Reuse instruction-flow lift storage within each decoding worker

docs/architecture.md

@@ -1036,6 +1036,14 @@ remain the same.
 Core callers opt in through `PipelineOptions::AnalysisCache` and must reset it
 before input or policy mutation; image pointers alone do not establish validity.
 
+Instruction-flow sweeps keep one `InstructionFlowScratch` per decoding worker.
+The workspace reuses only the temporary LowOp vector's storage: operations
+are cleared and the x87 state reset before every instruction, including after
+an unsupported lift. Returned references and transfer facts own their values.
+Disassembly pages own a workspace for that call; parallel access-width,
+reference and string-reference scans retire theirs with each worker. No TLS,
+session cache, new semantic shortcut or cross-instruction state is involved.
+
 Function selection also retains decoded image data-access widths. Their exact
 ordered function address/size inventory is checked after synchronization;
 changed extents rebuild the widths and discard source using the former names.

lib/sdk/capi/InstructionFlow.cpp

@@ -120,8 +120,16 @@ StackMove stackMove(const TargetRegInfo &Registers, llvm::ArrayRef<LowOp> Ops) {
 InstructionFlow summarizeInstructionFlow(const BinaryImage &Img, Decoder &Dec,
                                          const DecodedInsn &DI,
                                          const TargetRegInfo *StackRegisters) {
+  InstructionFlowScratch Scratch;
+  return Scratch.summarize(Img, Dec, DI, StackRegisters);
+}
+
+InstructionFlow
+InstructionFlowScratch::summarize(const BinaryImage &Img, Decoder &Dec,
+                                  const DecodedInsn &DI,
+                                  const TargetRegInfo *StackRegisters) {
   InstructionFlow Flow;
-  std::vector<LowOp> Ops;
+  Ops.clear();
   // Each row is lifted independently of its neighbours.
   Dec.resetX86FpuState();
   try {
@@ -352,7 +360,8 @@ constexpr uint64_t MaxUnsizedInstructions = 65536;
 /// direct jumps reach from \p Entry, short of another function's entry in
 /// \p Entries, in address order.  32-bit ARM, whose jumps can change the
 /// instruction set, reads up to the first terminator instead.
-std::vector<va_t> unsizedBody(const BinaryImage &Img, Decoder &Dec, va_t Entry,
+std::vector<va_t> unsizedBody(const BinaryImage &Img, Decoder &Dec,
+                              InstructionFlowScratch &Scratch, va_t Entry,
                               llvm::ArrayRef<va_t> Entries) {
   const auto OtherEntry = [&](va_t Address) {
     return Address != Entry &&
@@ -370,7 +379,7 @@ std::vector<va_t> unsizedBody(const BinaryImage &Img, Decoder &Dec, va_t Entry,
           if (OtherEntry(DI.Addr) || !Body.insert(DI.Addr).second)
             return false;
           if (FollowsJumps) {
-            const InstructionFlow Flow = summarizeInstructionFlow(Img, Dec, DI);
+            const InstructionFlow Flow = Scratch.summarize(Img, Dec, DI);
             if ((Flow.Kind == "jump" || Flow.Kind == "cjump") &&
                 Flow.Target != InvalidVA && !OtherEntry(Flow.Target) &&
                 !Body.count(Flow.Target))
@@ -387,7 +396,8 @@ std::vector<va_t> unsizedBody(const BinaryImage &Img, Decoder &Dec, va_t Entry,
 bool decodeExtents(
     const BinaryImage &Img, llvm::ArrayRef<std::pair<va_t, uint64_t>> Extents,
     llvm::ArrayRef<va_t> Entries,
-    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &)>
+    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &,
+                            InstructionFlowScratch &)>
         Visit) {
   // The parts of a function its unwind records keep apart from its body,
   // past other functions' code, are its code too: MSVC moves cold paths
@@ -402,21 +412,23 @@ bool decodeExtents(
       DecoderFailed = true;
       return;
     }
+    InstructionFlowScratch Scratch;
     for (size_t Index = Claim(); Index < Total; Index = Claim()) {
       const auto [Entry, Size] = Extents[Index];
       if (Size > 0) {
         (void)decodeNativeRange(
             Img, Local, Entry, Size, Size,
             [&](const DecodedInsn &DI, const uint8_t *, int) {
-              Visit(Index, Local, DI);
+              Visit(Index, Local, DI, Scratch);
               return true;
             });
       } else {
-        for (const va_t Address : unsizedBody(Img, Local, Entry, Entries))
+        for (const va_t Address :
+             unsizedBody(Img, Local, Scratch, Entry, Entries))
           (void)decodeNativeRange(
               Img, Local, Address, 0, 1,
               [&](const DecodedInsn &DI, const uint8_t *, int) {
-                Visit(Index, Local, DI);
+                Visit(Index, Local, DI, Scratch);
                 return true;
               });
       }
@@ -426,7 +438,7 @@ bool decodeExtents(
               Img, Local, Range.Begin, Range.End - Range.Begin,
               Range.End - Range.Begin,
               [&](const DecodedInsn &DI, const uint8_t *, int) {
-                Visit(Index, Local, DI);
+                Visit(Index, Local, DI, Scratch);
                 return true;
               });
     }
@@ -442,7 +454,8 @@ bool isDataAccess(const InstructionRef &Ref) {
 
 bool decodeFunctions(
     const BinaryImage &Img, const FunctionPage &Page,
-    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &)>
+    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &,
+                            InstructionFlowScratch &)>
         Visit) {
   std::vector<std::pair<va_t, uint64_t>> Extents;
   Extents.reserve(Page.Functions.size());
@@ -463,9 +476,10 @@ collectDataAccessWidths(const BinaryImage &Img,
     std::vector<std::vector<std::pair<va_t, unsigned>>> Slots(
         Page.Functions.size());
     const bool Decoded = decodeFunctions(
-        Img, Page, [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
-          for (const InstructionRef &Ref :
-               summarizeInstructionFlow(Img, Dec, DI).Refs)
+        Img, Page,
+        [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+            InstructionFlowScratch &Scratch) {
+          for (const InstructionRef &Ref : Scratch.summarize(Img, Dec, DI).Refs)
             if (isDataAccess(Ref))
               Slots[Index].emplace_back(Ref.To, Ref.Bytes);
         });
@@ -513,9 +527,10 @@ DataAccessScan::update(const BinaryImage &Img,
   std::vector<std::vector<std::pair<va_t, unsigned>>> Slots(Read.size());
   if (!Read.empty() &&
       !decodeExtents(Img, Read, NextEntries,
-                     [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
+                     [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+                         InstructionFlowScratch &Scratch) {
                        for (const InstructionRef &Ref :
-                            summarizeInstructionFlow(Img, Dec, DI).Refs)
+                            Scratch.summarize(Img, Dec, DI).Refs)
                          if (isDataAccess(Ref))
                            Slots[Index].emplace_back(Ref.To, Ref.Bytes);
                      })) {

lib/sdk/capi/InstructionFlow.h

@@ -124,6 +124,19 @@ summarizeInstructionFlow(const BinaryImage &Img, Decoder &Dec,
                          const DecodedInsn &DI,
                          const TargetRegInfo *StackRegisters = nullptr);
 
+/// One decoding worker's temporary lift storage. Only capacity survives a
+/// call: every instruction starts with empty operations and fresh FPU state.
+/// Results own their facts and never borrow this workspace.
+class InstructionFlowScratch {
+public:
+  InstructionFlow summarize(const BinaryImage &Img, Decoder &Dec,
+                            const DecodedInsn &DI,
+                            const TargetRegInfo *StackRegisters = nullptr);
+
+private:
+  std::vector<LowOp> Ops;
+};
+
 /// The pointer the loader stored in a relocated slot, as a code address when
 /// the slot points to code.
 std::optional<va_t> relocatedPointer(const BinaryImage &Img, va_t Slot);
@@ -144,18 +157,19 @@ FunctionPage functionPage(llvm::ArrayRef<FuncInfo> Functions, va_t FirstEntry,
 FunctionPage functionPage(const Session &S, va_t FirstEntry, int MaxFunctions);
 
 /// Decodes each function of \p Page in \p Img with a decoder of its own,
-/// functions in parallel, and calls \p Visit(Index, Decoder, Instruction) for
-/// each of its instructions in address order: up to the function's recorded
-/// size, or, when it has none, the instructions its own direct jumps reach
-/// from its entry short of another function's entry (Page.Entries; on 32-bit
-/// ARM, up to its first terminator), then the parts of it that lie apart
-/// from its body (ExceptionInfo::outlyingChunks).  One function's visits run
-/// on one thread, so writing only to its own output keeps the result
-/// independent of the thread count.  False when a decoder could not be set
-/// up.
+/// functions in parallel, and calls Visit(Index, Decoder, Instruction, Scratch)
+/// with one exclusive scratch workspace per decoding worker for each of its
+/// instructions in address order: up to the function's recorded size, or,
+/// when it has none, the instructions its own direct jumps reach from its
+/// entry short of another function's entry (Page.Entries; on 32-bit ARM, up
+/// to its first terminator), then the parts of it that lie apart from its
+/// body (ExceptionInfo::outlyingChunks).  One function's visits run on one
+/// thread, so writing only to its own output keeps the result independent of
+/// the thread count.  False when a decoder could not be set up.
 bool decodeFunctions(
     const BinaryImage &Img, const FunctionPage &Page,
-    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &)>
+    llvm::function_ref<void(size_t Index, Decoder &, const DecodedInsn &,
+                            InstructionFlowScratch &)>
         Visit);
 
 /// The width of the reads and writes the instructions of each of \p

lib/sdk/capi/NeverDCAPICallPaths.cpp

@@ -201,13 +201,14 @@ std::shared_ptr<const CallReferenceGraph> buildGraph(Session &S) {
   };
   std::vector<std::vector<Reference>> Slots(Bodies.size());
   // The references of one instruction of body \p Index.
-  const auto Collect = [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
+  const auto Collect = [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+                           InstructionFlowScratch &Scratch) {
     auto &Out = Slots[Index];
     const auto Add = [&](std::optional<uint32_t> To, uint8_t Kind) {
       if (To)
         Out.push_back({*To, Kind, DI.Addr});
     };
-    const InstructionFlow Flow = summarizeInstructionFlow(Img, Dec, DI);
+    const InstructionFlow Flow = Scratch.summarize(Img, Dec, DI);
     const bool Indirect = Flow.Kind == "icall" || Flow.Kind == "ijump";
     if (Flow.Target != InvalidVA && !Flow.Kind.empty() &&
         Flow.Kind != UnliftedFlow) {
@@ -251,10 +252,11 @@ std::shared_ptr<const CallReferenceGraph> buildGraph(Session &S) {
         Bodies.size(), First + FunctionPage::MaxFunctionsPerPage);
     Page.Functions.assign(Bodies.begin() + First, Bodies.begin() + Last);
     Page.Entries = Entries;
-    if (!decodeFunctions(
-            Img, Page, [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
-              Collect(First + Index, Dec, DI);
-            })) {
+    if (!decodeFunctions(Img, Page,
+                         [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+                             InstructionFlowScratch &Scratch) {
+                           Collect(First + Index, Dec, DI, Scratch);
+                         })) {
       S.setError("failed to initialize a decoder for the image");
       return nullptr;
     }

lib/sdk/capi/NeverDCAPIDisasm.cpp

@@ -226,6 +226,7 @@ const char *neverd_disasm_json_ex(neverd_session_t Sess, neverd_va_t Addr,
       MaxInsns > 0 ? static_cast<uint64_t>(MaxInsns) : (Span > 0 ? Span : 256);
 
   const TargetRegInfo &Registers = getTargetRegInfo(S->Img.Arch);
+  InstructionFlowScratch Scratch;
   const bool ModeKnown = decodeNativeRange(
       S->Img, S->Dec, Addr, Span, Limit,
       [&](const DecodedInsn &DI, const uint8_t *Bytes, int Sz) {
@@ -255,7 +256,7 @@ const char *neverd_disasm_json_ex(neverd_session_t Sess, neverd_va_t Addr,
           Obj["operand_bits"] = std::move(Bits);
         }
         if (Options & (NEVERD_DISASM_FLOW | NEVERD_DISASM_STACK)) {
-          const InstructionFlow Flow = summarizeInstructionFlow(
+          const InstructionFlow Flow = Scratch.summarize(
               S->Img, S->Dec, DI,
               Options & NEVERD_DISASM_STACK ? &Registers : nullptr);
           if (Options & NEVERD_DISASM_FLOW) {
@@ -328,9 +329,11 @@ const char *neverd_code_refs_json(neverd_session_t Sess, neverd_va_t FirstEntry,
   // Each function's references, merged in entry order.
   std::vector<std::vector<Ref>> Slots(Page.Functions.size());
   const bool Decoded = decodeFunctions(
-      S->Img, Page, [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
+      S->Img, Page,
+      [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+          InstructionFlowScratch &Scratch) {
         auto &Out = Slots[Index];
-        const InstructionFlow Flow = summarizeInstructionFlow(S->Img, Dec, DI);
+        const InstructionFlow Flow = Scratch.summarize(S->Img, Dec, DI);
         if (Flow.Target != InvalidVA && !Flow.Kind.empty() &&
             Flow.Kind != UnliftedFlow)
           Out.push_back({DI.Addr, Flow.Target, Flow.Kind});

lib/sdk/capi/NeverDCAPIStringRefs.cpp

@@ -89,8 +89,10 @@ const char *neverd_string_refs_json(neverd_session_t Sess,
   std::vector<std::vector<Row>> Slots(Page.Functions.size());
   const BinaryImage &Img = S->Img;
   const bool Decoded = decodeFunctions(
-      Img, Page, [&](size_t Index, Decoder &Dec, const DecodedInsn &DI) {
-        const InstructionFlow Flow = summarizeInstructionFlow(Img, Dec, DI);
+      Img, Page,
+      [&](size_t Index, Decoder &Dec, const DecodedInsn &DI,
+          InstructionFlowScratch &Scratch) {
+        const InstructionFlow Flow = Scratch.summarize(Img, Dec, DI);
         // The instruction as neverd_disasm_json() spells it.
         const auto instruction = [&] {
           std::string Text = DI.Raw ? DI.Raw->mnemonic : "";

tools/neverd-worker/CMakeLists.txt

@@ -244,6 +244,10 @@ if(BUILD_TESTING OR NEVERD_BUILD_TESTS)
           COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/tests/data_presentation_cli_test.py"
                   "$<TARGET_FILE:neverd-worker>" "${_neverd_worker_test_cli}" --engine "$<TARGET_FILE:neverd_shared>")
         set_tests_properties(NeverDWorkerCLIDataPresentation PROPERTIES TIMEOUT 180)
+        add_test(NAME NeverDWorkerCLIInstructionFlow
+          COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/tests/instruction_flow_cli_test.py"
+                  "$<TARGET_FILE:neverd-worker>" "${_neverd_worker_test_cli}")
+        set_tests_properties(NeverDWorkerCLIInstructionFlow PROPERTIES TIMEOUT 120)
         add_test(NAME NeverDWorkerCLIStackFrame
           COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/tests/stack_frame_cli_test.py"
                   "$<TARGET_FILE:neverd-worker>" "${_neverd_worker_test_cli}" --cc "${CMAKE_C_COMPILER}")

tools/neverd-worker/tests/instruction_flow_cli_test.py

@@ -0,0 +1,100 @@
+#!/usr/bin/env python3
+"""CLI and worker reference pages agree on synthetic instruction sweeps."""
+import argparse
+import json
+from pathlib import Path
+import struct
+import subprocess
+import tempfile
+
+from transport_test import Client
+
+ENTRY = 0x400078
+
+
+def fixture(count):
+    # Explicit symbol extents avoid depending on a detector heuristic. Both
+    # 8-byte and 4-byte loads are followed by NOP/RET: scratch reuse must not
+    # give those trailing instructions a preceding instruction's reference.
+    target = ENTRY + 16 * count
+    code = bytearray()
+    expected = []
+    names = bytearray(b"\0")
+    symbols = bytearray(24)
+    for index in range(count):
+        start = ENTRY + 16 * index
+        load = bytes.fromhex("488b05" if index % 2 == 0 else "8b05")
+        body = load + struct.pack("<i", target - start - len(load) - 4) + bytes.fromhex("90c3")
+        code += body + b"\xcc" * (16 - len(body))
+        symbols += struct.pack("<IBBHQQ", len(names), 0x12, 0, 1, start, len(body))
+        names += f"fixture_{index}".encode() + b"\0"
+        expected.append((start, target, "read", 8 if index % 2 == 0 else 4))
+    data = b"fixture text\0"
+    image = bytearray(120) + code + data
+
+    def append(contents, alignment=1):
+        image.extend(bytes((-len(image)) % alignment))
+        offset = len(image)
+        image.extend(contents)
+        return offset
+
+    symbol_offset = append(symbols, 8)
+    name_offset = append(names)
+    section_names = b"\0.text\0.rodata\0.symtab\0.strtab\0.shstrtab\0"
+    section_name_offset = append(section_names)
+    sections = bytes(64)
+    for name, kind, flags, address, offset, size, link, info, alignment, entry_size in [
+        (1, 1, 6, ENTRY, 120, len(code), 0, 0, 16, 0),
+        (7, 1, 2, target, 120 + len(code), len(data), 0, 0, 1, 0),
+        (15, 2, 0, 0, symbol_offset, len(symbols), 4, 1, 8, 24),
+        (23, 3, 0, 0, name_offset, len(names), 0, 0, 1, 0),
+        (31, 3, 0, 0, section_name_offset, len(section_names), 0, 0, 1, 0),
+    ]:
+        sections += struct.pack("<IIQQQQIIQQ", name, kind, flags, address, offset, size,
+                                link, info, alignment, entry_size)
+    section_offset = append(sections, 8)
+    image[:64] = struct.pack("<16sHHIQQQIHHHHHH", b"\x7fELF\x02\x01\x01" + bytes(9),
+                             2, 62, 1, ENTRY, 64, section_offset, 0, 64, 56, 1, 64, 6, 5)
+    mapped = 120 + len(code) + len(data)
+    image[64:120] = struct.pack("<IIQQQQQQ", 1, 5, 0, 0x400000, 0x400000, mapped, mapped, 4096)
+    return image, expected
+
+
+def run(worker, cli):
+    with tempfile.TemporaryDirectory(prefix="neverd-flow-cli-") as directory:
+        for count in (2, 2049):
+            binary = Path(directory) / f"functions-{count}.elf"
+            data, expected = fixture(count)
+            binary.write_bytes(data)
+            target = expected[0][1]
+            for _ in range(2):
+                result = subprocess.run([cli, "xrefs", str(binary), "--addr", hex(target),
+                                         "--no-debug", "--json"], capture_output=True,
+                                        text=True, timeout=60)
+                assert result.returncode == 0, result.stdout + result.stderr
+                rows = json.loads(result.stdout)
+                actual = [(int(r["from"], 16), int(r["to"], 16), r["kind"], r.get("size", 0))
+                          for r in rows]
+                assert actual == expected, (count, len(actual), actual[:4])
+            with Client(worker) as client:
+                opened = client.call("open", {"path": str(binary), "analysis": False, "debug_info": False})
+                assert opened["status"] == "ok", opened
+                actual = []
+                while len(actual) < count:
+                    reply = client.call("xrefs", {"address": hex(target), "offset": len(actual), "limit": 512})
+                    assert reply["status"] == "ok", reply
+                    page = reply["payload"]
+                    assert page["total"] == count, page
+                    assert page["items"], page
+                    actual.extend((int(r["from"], 16), int(r["to"], 16), r["kind"])
+                                  for r in page["items"])
+                assert actual == [r[:3] for r in expected]
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser()
+    parser.add_argument("worker")
+    parser.add_argument("cli")
+    args = parser.parse_args()
+    run(args.worker, args.cli)
+    print("CLI/worker references, access widths and complete multi-page inventories passed")