diff --git a/bridge/ghidra/ReaGhidraBridge.java b/bridge/ghidra/ReaGhidraBridge.java index 7ec7a205c..5da3e1374 100644 --- a/bridge/ghidra/ReaGhidraBridge.java +++ b/bridge/ghidra/ReaGhidraBridge.java @@ -80,12 +80,14 @@ import ghidra.program.database.mem.FileBytes; import ghidra.program.model.address.SegmentedAddress; import ghidra.program.model.reloc.Relocation; import java.io.ByteArrayOutputStream; +import ghidra.program.model.symbol.Namespace; import ghidra.program.model.symbol.RefType; import ghidra.program.model.symbol.Reference; import ghidra.program.model.symbol.ReferenceIterator; import ghidra.program.model.symbol.SourceType; import ghidra.program.model.symbol.Symbol; import ghidra.program.model.symbol.SymbolIterator; +import ghidra.program.model.symbol.SymbolTable; import ghidra.program.model.pcode.FunctionPrototype; import ghidra.program.model.pcode.HighFunction; import ghidra.program.model.pcode.HighParam; @@ -1434,18 +1436,22 @@ public final class ReaGhidraBridge extends HeadlessScript { boolean commit = false; try { Function function = currentProgram.getFunctionManager().getFunctionAt(address); - String expected = name; + SymbolTable table = currentProgram.getSymbolTable(); + Symbol primary = function == null ? table.getPrimarySymbol(address) : function.getSymbol(); + Namespace namespace = primary == null + ? currentProgram.getGlobalNamespace() + : primary.getParentNamespace(); + String expected = primary == null ? name : symbolLeafName(primary, name); + for (Symbol symbol : table.getLabelOrFunctionSymbols(expected, namespace)) { + if (!address.equals(symbol.getAddress())) return false; + } if (function != null) { - expected = symbolLeafName(function.getSymbol(), name); function.setName(expected, SourceType.USER_DEFINED); } else { - var table = currentProgram.getSymbolTable(); - Symbol primary = table.getPrimarySymbol(address); 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); + Symbol created = table.createLabel(address, expected, SourceType.USER_DEFINED); created.setPrimary(); } } diff --git a/scripts/lib/real-ghidra-namespace-annotations.mjs b/scripts/lib/real-ghidra-namespace-annotations.mjs index f67343526..c65c02e87 100644 --- a/scripts/lib/real-ghidra-namespace-annotations.mjs +++ b/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 verifyDuplicateAddressNames(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,8 @@ export async function verifyGhidraNamespaceAnnotations({ }); assert.deepEqual(unchanged.annotations, qualified.annotations); } - await verifyAddressNames(call, [entries[0], entries[2], ...globals]); + await verifyAddressNames(call, [entries[0], entries[2]], "function"); + await verifyAddressNames(call, globals, "label"); for (const { namespace, address } of entries) { assert.equal( await call("set_address_name", { @@ -177,24 +179,107 @@ export async function verifyGhidraNamespaceAnnotations({ } } -async function verifyAddressNames(call, symbols) { - for (const { namespace, address } of symbols) { +async function verifyDuplicateAddressNames(call, procedures, names) { + const functionEntries = [ + "rea_duplicate_function_a", + "rea_duplicate_function_b", + ].map((name) => { + const procedure = procedures.find( + ({ value }) => value === name || value === `_${name}`, + ); + assert.ok(procedure, `Missing duplicate-name fixture function ${name}`); + return { address: procedure.address, original: procedure.value }; + }); + const labelEntries = ["rea_duplicate_label_a", "rea_duplicate_label_b"].map( + (name) => { + const symbol = names.find( + (item) => + (item.value === name || item.value === `_${name}`) && + item.symbol.primary, + ); + assert.ok(symbol, `Missing duplicate-name fixture label ${name}`); + return { address: symbol.address, original: symbol.value }; + }, + ); + + for (const [kind, entries] of [ + ["function", functionEntries], + ["label", labelEntries], + ]) { + const [owner, duplicate] = entries; + const name = `rea_duplicate_${kind}`; 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: owner.address, name }), + true, + "Renaming the current owner to its existing name remains idempotent", + ); + assert.equal( + await call("set_address_name", { address: duplicate.address, name }), + false, + ); + assert.equal( + await call("address_name", { address: duplicate.address }), + duplicate.original, + "A rejected duplicate leaves the other address unchanged", + ); + + const batchName = `rea_batch_duplicate_${kind}`; + assert.deepEqual( + await call("set_addresses_names", { + names: { + [owner.address]: batchName, + [duplicate.address]: batchName, + }, + }), + { [owner.address]: true, [duplicate.address]: false }, + ); + assert.equal( + await call("address_name", { address: owner.address }), + batchName, + ); + assert.equal( + await call("address_name", { address: duplicate.address }), + duplicate.original, + "A failed batch member rolls back only its own address", + ); + const owners = (await call("list_names")).filter( + (item) => item.value === batchName && item.symbol.primary, + ); + assert.deepEqual( + owners.map(({ address }) => address), + [owner.address], + ); + } +} + +async function verifyAddressNames(call, symbols, kind) { + for (const { namespace, address } 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, ); assert.equal(await call("address_name", { address }), qualified); assert.equal( - await call("set_address_name", { address, name: "other::literal" }), + await call("set_address_name", { + address, + name: `other::literal_${kind}`, + }), true, ); const literal = await call("address_name", { address }); - assert.equal(literal, `${namespace}::other::literal`); + assert.equal(literal, `${namespace}::other::literal_${kind}`); assert.equal( await call("set_address_name", { address, name: literal }), true, @@ -235,7 +320,7 @@ async function verifyAddressNames(call, symbols) { assert.deepEqual( await call("set_addresses_names", { names: { - [valid.address]: `${valid.namespace}::batch_renamed`, + [valid.address]: `${valid.namespace}::batch_renamed_${kind}`, [invalid.address]: `${invalid.namespace}::`, }, }), @@ -243,18 +328,18 @@ async function verifyAddressNames(call, symbols) { ); assert.equal( await call("address_name", { address: valid.address }), - `${valid.namespace}::batch_renamed`, + `${valid.namespace}::batch_renamed_${kind}`, ); assert.equal( await call("address_name", { address: invalid.address }), - `${invalid.namespace}::address_qualified`, + `${invalid.namespace}::address_qualified_${kind}`, ); const names = await call("list_names"); for (const { namespace, address } of symbols) { const expected = address === valid.address - ? `${namespace}::batch_renamed` - : `${namespace}::address_qualified`; + ? `${namespace}::batch_renamed_${kind}` + : `${namespace}::address_qualified_${kind}`; assert.ok( names.some( (item) => diff --git a/tests/conformance/ghidra/namespaces.c b/tests/conformance/ghidra/namespaces.c index c672697c3..6530c4048 100644 --- a/tests/conformance/ghidra/namespaces.c +++ b/tests/conformance/ghidra/namespaces.c @@ -10,6 +10,17 @@ volatile int rea_namespace_alpha_value volatile int rea_namespace_nested_value __asm__(REA_CPP_SYMBOL("_ZN5outer5inner5valueE")) = 1; +volatile int rea_duplicate_label_a = 3; +volatile int rea_duplicate_label_b = 4; + +__attribute__((noinline, used)) int rea_duplicate_function_a(int value) { + return value + 1; +} + +__attribute__((noinline, used)) int rea_duplicate_function_b(int value) { + return value + 2; +} + __attribute__((noinline, used)) int rea_namespace_alpha(int value) __asm__(REA_CPP_SYMBOL("_ZN5alpha4sameEi")); __attribute__((noinline, used)) int rea_namespace_beta(int value)