fix(ghidra): preserve explicit annotation naming semantics

bridge/ghidra/ReaGhidraBridge.java

@@ -1105,6 +1105,17 @@ public final class ReaGhidraBridge extends HeadlessScript {
         return requested;
     }
 
+    private void requireAvailableAddressName(Address address,
+            ghidra.program.model.symbol.Namespace namespace, String name)
+            throws ghidra.util.exception.DuplicateNameException {
+        for (Symbol owner : currentProgram.getSymbolTable().getSymbols(name, namespace)) {
+            if (!owner.getAddress().equals(Address.NO_ADDRESS) && !owner.getAddress().equals(address))
+                throw new ghidra.util.exception.DuplicateNameException(
+                    "Symbol name " + name + " is already owned at " +
+                    canonicalAddress(owner.getAddress()) + " in namespace " + namespace.getName(true));
+        }
+    }
+
     private static String requireText(JsonObject params, String field) {
         JsonElement value = params.get(field);
         if (value == null || !value.isJsonPrimitive() || !value.getAsJsonPrimitive().isString())
@@ -1434,20 +1445,19 @@ public final class ReaGhidraBridge extends HeadlessScript {
         boolean commit = false;
         try {
             Function function = currentProgram.getFunctionManager().getFunctionAt(address);
-            String expected = name;
+            var table = currentProgram.getSymbolTable();
+            Symbol primary = function == null ? table.getPrimarySymbol(address) : function.getSymbol();
+            boolean existing = primary != null && !primary.isDynamic();
+            String expected = existing ? symbolLeafName(primary, name) : name;
+            requireAvailableAddressName(address,
+                existing ? primary.getParentNamespace() : currentProgram.getGlobalNamespace(), expected);
             if (function != null) {
-                expected = symbolLeafName(function.getSymbol(), name);
                 function.setName(expected, SourceType.USER_DEFINED);
+            } else if (existing) {
+                primary.setName(expected, SourceType.USER_DEFINED);
             } else {
-                var table = currentProgram.getSymbolTable();
+                Symbol created = table.createLabel(address, expected, SourceType.USER_DEFINED);
-                Symbol primary = table.getPrimarySymbol(address);
+                created.setPrimary();
-                if (primary != null && !primary.isDynamic()) {
-                    expected = symbolLeafName(primary, name);
-                    primary.setName(expected, SourceType.USER_DEFINED);
-                } else {
-                    Symbol created = table.createLabel(address, name, SourceType.USER_DEFINED);
-                    created.setPrimary();
-                }
             }
             invalidateAnalysisCaches();
             Symbol readback = currentProgram.getSymbolTable().getPrimarySymbol(address);

docs/native-investigation.md

@@ -400,8 +400,11 @@ GUI authority, and REA never falls back automatically to Hopper.
 `set_address_name` and `set_addresses_names` give Ghidra sessions the same
 compact naming results as Hopper: a local function entry renames that function,
 and any other mapped address, such as a global, receives a new or renamed primary
-label. A batch applies each address separately and reports one success boolean
+label. Names owned by another address in the same namespace are rejected;
-per address. Use them to name many globals and helpers cheaply; use
+the same leaf name remains valid in a different namespace. A batch applies each
+address separately and reports one success boolean per address, preserving the
+other entries when one fails. Ghidra batches do not move or swap names. Use them
+to name many globals and helpers cheaply; use
 `annotate_native_function` when you also want comments and a refreshed dossier.
 Ghidra name edits preserve the existing namespace. Supply either a leaf name
 such as `renamed` or a fully qualified name in that namespace, such as

scripts/lib/real-ghidra-namespace-annotations.mjs

@@ -62,6 +62,7 @@ export async function verifyGhidraNamespaceAnnotations({
       assert.ok(symbol, `Missing demangled fixture global ${namespace}::value`);
       return { namespace, address: symbol.address };
     });
+    await verifyAddressNameCollisions(call, procedures, names);
     const ambiguous = await reject("procedure_address", { procedure: "same" });
     assert.equal(ambiguous.code, "invalid_request");
     for (const { address } of entries)
@@ -133,7 +134,11 @@ export async function verifyGhidraNamespaceAnnotations({
       });
       assert.deepEqual(unchanged.annotations, qualified.annotations);
     }
-    await verifyAddressNames(call, [entries[0], entries[2], ...globals]);
+    await verifyAddressNames(call, [
+      { ...entries[0], kind: "function" },
+      { ...entries[2], kind: "function" },
+      ...globals.map((symbol) => ({ ...symbol, kind: "label" })),
+    ]);
     for (const { namespace, address } of entries) {
       assert.equal(
         await call("set_address_name", {
@@ -177,13 +182,141 @@ export async function verifyGhidraNamespaceAnnotations({
   }
 }
 
-async function verifyAddressNames(call, symbols) {
+async function verifyAddressNameCollisions(call, procedures, names) {
-  for (const { namespace, address } of symbols) {
+  const find = (items, name) => {
+    const symbol = items.find(
+      (item) => item.value === name || item.value === `_${name}`,
+    );
+    assert.ok(symbol, `Missing naming collision fixture ${name}`);
+    return symbol;
+  };
+  const first = find(procedures, "rea_name_first");
+  const second = find(procedures, "rea_name_second");
+  const firstLabel = find(names, "rea_name_data_first");
+  const secondLabel = find(names, "rea_name_data_second");
+  for (const [owner, other, name] of [
+    [first, second, "owned_function"],
+    [firstLabel, secondLabel, "owned_label"],
+  ]) {
     assert.equal(
-      await call("set_address_name", { address, name: "address_renamed" }),
+      await call("set_address_name", { address: owner.address, name }),
       true,
     );
-    const qualified = `${namespace}::address_qualified`;
+    assert.equal(
+      await call("set_address_name", { address: other.address, name }),
+      false,
+      "A different address must not acquire the existing name",
+    );
+    assert.equal(
+      await call("address_name", { address: other.address }),
+      other.value,
+    );
+    assert.equal(
+      await call("set_address_name", { address: owner.address, name }),
+      true,
+      "The owning address may repeat the same name",
+    );
+  }
+  for (const [address, name, original] of [
+    [first.address, "owned_label", "owned_function"],
+    [firstLabel.address, "owned_function", "owned_label"],
+  ]) {
+    assert.equal(await call("set_address_name", { address, name }), false);
+    assert.equal(await call("address_name", { address }), original);
+  }
+  const alphaFunction = find(procedures, "alpha::same");
+  const alphaLabel = find(names, "alpha::value");
+  for (const name of ["value", "alpha::value"]) {
+    assert.equal(
+      await call("set_address_name", { address: alphaFunction.address, name }),
+      false,
+    );
+    assert.equal(
+      await call("address_name", { address: alphaFunction.address }),
+      alphaFunction.value,
+    );
+  }
+  assert.equal(
+    await call("set_address_name", {
+      address: alphaLabel.address,
+      name: "same",
+    }),
+    false,
+  );
+  assert.equal(
+    await call("address_name", { address: alphaLabel.address }),
+    alphaLabel.value,
+  );
+  assert.deepEqual(
+    await call("set_addresses_names", {
+      names: {
+        [secondLabel.address]: "batch_available",
+        [second.address]: "owned_function",
+        [firstLabel.address]: "owned_label",
+      },
+    }),
+    {
+      [secondLabel.address]: true,
+      [second.address]: false,
+      [firstLabel.address]: true,
+    },
+  );
+  assert.equal(
+    await call("address_name", { address: second.address }),
+    second.value,
+  );
+  assert.equal(
+    await call("address_name", { address: first.address }),
+    "owned_function",
+  );
+  assert.equal(
+    await call("address_name", { address: secondLabel.address }),
+    "batch_available",
+  );
+  const before = await call("annotate_native_function", {
+    procedure: second.address,
+    comment: "Keep this comment",
+    inline_comment: "Keep this inline comment",
+  });
+  assert.equal(
+    await call("set_address_name", {
+      address: second.address,
+      name: "owned_function",
+    }),
+    false,
+  );
+  const unchanged = await call("annotate_native_function", {
+    procedure: second.address,
+    name: second.value,
+  });
+  assert.deepEqual(unchanged.annotations, before.annotations);
+  assert.equal(
+    await call("set_address_name", {
+      address: first.address,
+      name: "released_function",
+    }),
+    true,
+  );
+  assert.equal(
+    await call("set_address_name", {
+      address: second.address,
+      name: "owned_function",
+    }),
+    true,
+    "A name becomes available after its old owner releases it",
+  );
+}
+
+async function verifyAddressNames(call, symbols) {
+  for (const { namespace, address, kind } of symbols) {
+    assert.equal(
+      await call("set_address_name", {
+        address,
+        name: `address_renamed_${kind}`,
+      }),
+      true,
+    );
+    const qualified = `${namespace}::address_qualified_${kind}`;
     assert.equal(
       await call("set_address_name", { address, name: qualified }),
       true,
@@ -247,14 +380,14 @@ async function verifyAddressNames(call, symbols) {
   );
   assert.equal(
     await call("address_name", { address: invalid.address }),
-    `${invalid.namespace}::address_qualified`,
+    `${invalid.namespace}::address_qualified_${invalid.kind}`,
   );
   const names = await call("list_names");
-  for (const { namespace, address } of symbols) {
+  for (const { namespace, address, kind } of symbols) {
     const expected =
       address === valid.address
         ? `${namespace}::batch_renamed`
-        : `${namespace}::address_qualified`;
+        : `${namespace}::address_qualified_${kind}`;
     assert.ok(
       names.some(
         (item) =>

src/ghidra/GhidraProviderCapabilities.ts

@@ -55,8 +55,8 @@ export const limitationsFor = (operation: string): readonly string[] => {
     case "set_addresses_names":
       return [
         ...common,
-        "Names use Ghidra USER_DEFINED source. A local function entry renames that function; any other mapped address receives a new or renamed primary label. Existing symbols retain their namespace and accept leaf names or qualified names within that namespace; qualified readback can be reused without adding namespace prefixes. Readback compares the primary symbol name.",
+        "Names use Ghidra USER_DEFINED source. A local function entry renames that function; any other mapped address receives a new or renamed primary label. Existing symbols retain their namespace and accept leaf names or qualified names within that namespace; qualified readback can be reused without adding namespace prefixes. Names owned by another address in that namespace are rejected; the same leaf in a different namespace remains valid. Readback compares the primary symbol name.",
-        "set_addresses_names applies each address in its own transaction and reports per-address success; invalid names, unmapped addresses, and readback mismatches roll back only that address.",
+        "set_addresses_names applies each address in its own transaction and reports per-address success; invalid or occupied names, unmapped addresses, and readback mismatches roll back only that address. Names are not moved or swapped between addresses.",
         "Metadata edits invalidate immutable analysis snapshots and are discarded on close; this is not a saved Ghidra project.",
       ];
     case "inspect_native_load_image":

tests/conformance/ghidra/namespaces.c

@@ -9,6 +9,15 @@ volatile int rea_namespace_alpha_value
     __asm__(REA_CPP_SYMBOL("_ZN5alpha5valueE")) = 3;
 volatile int rea_namespace_nested_value
     __asm__(REA_CPP_SYMBOL("_ZN5outer5inner5valueE")) = 1;
+volatile int rea_name_data_first = 5;
+volatile int rea_name_data_second = 9;
+
+__attribute__((noinline, used)) int rea_name_first(int value) {
+  return value + rea_name_data_first;
+}
+__attribute__((noinline, used)) int rea_name_second(int value) {
+  return value + rea_name_data_second;
+}
 
 __attribute__((noinline, used)) int rea_namespace_alpha(int value)
     __asm__(REA_CPP_SYMBOL("_ZN5alpha4sameEi"));
@@ -23,5 +32,5 @@ int rea_namespace_nested(int value) { return value - rea_namespace_nested_value;
 
 int main(void) {
   return rea_namespace_alpha(1) + rea_namespace_beta(2) +
-         rea_namespace_nested(3);
+         rea_namespace_nested(3) + rea_name_first(4) + rea_name_second(5);
 }