Use AddressSpace equals method for checks

[mikenawrocki]

I believe the .equals() method, rather than equality comparison via !=, should be used here and several other places in this file (and possibly in other places within the project):

https://github.com/NationalSecurityAgency/ghidra/blob/bf3fbbcb1b003b22f89a3795d5fa7a4c1d954c0d/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/util/AbstractStoredProgramContext.java#L251

I'm running into the AssertException below when the RegisterMergeManager is invoked on a shared project that uses Overlays. Using jdb, I can see the address spaces have the same names and IDs, .equals() returns true, but == is false. This causes merge to fail.

I'm using Ghidra 10.3 but I believe the bug is present on master currently.

[mikenawrocki]

An update: patching it locally to use .equals allows the merge to succeed in my instance.

[ghidra1]

Could you please provide some additional details:

1. Is this in relationto a multi-user shared project merge/checkin or a program Diff merge?
2. If using Diff merge does the source program use the exact same processor?
3. What processor are you using for the merged program?
4. Does your program contain instructions within Overlay Memory Blocks?

[mikenawrocki]

Sure thing:

1. Yes it's the multi-user shared project merge
2. N/A
3. I can't say here, but I think the issue is not processor specific. I could try replicating the behavior with e.g., ARM if that helps.
4. Yes it does

Also, this is cropping up I believe in the following sequence:

1. User A sets register values for region X
2. User B sets register values for region Y (in our case, different overlay regions)
3. User A checks in changes
4. User B attempts to merge, receives an AssertException

Thanks!

[ghidra1]

The issue may be a merge code issue which is suppose to work with equivalent addresses and not directly compare objects/addresses between two programs. I will need to check code in either case.

Please ignore my question about the source of the AssertException which you pointed to in your original post. Do you have more of the stack trace you can share? (i.e., where was remove method called from?)

[ghidra1]

The use of != is correct. The error is caused by improper use of the remove method which expects a contiguous address range within a single address space. If your switching this to a .equals solved the issue, this would imply that the range was formed with a start address from one program and an end address from another or some other abnormal situation. We should be relying on a single instance of an address space for a given program. Events that can trigger creation of new instances would be undo/redo or memory changes which should not be occuring durring a merge.

[mikenawrocki]

I recall it coming from here specifically:

https://github.com/NationalSecurityAgency/ghidra/blob/cae9190c13709fbd889edbe8f46d28c99df0a588/Ghidra/Features/Base/src/main/java/ghidra/app/merge/listing/RegisterMergeManager.java#L207

I don't have the original traceback handy since I locally patched the code and the merge succeeded, but may be able to reproduce it. I've had this specific issue crop up twice so far on this project.

This is on a single program, and in my instance, the start and end addresses were the same space (named after the overlay, with the same space ID), and the start came before the end, both within the valid range for the overlay.

I'm not entirely sure how I got different instances, it's possible that I created something, undid it, saved, then tried to merge.

[mikenawrocki]

I was able to pull the traceback from the debug log; the summary version is:

AbstractStoredProgramContext.java:252
ProgramRegisterContextDB.java:234
RegisterMergeManager.java:207
ProgramContextMergeManager.java:228
MergeManager.java:430
RunManager.java:334
...

[ghidra1]

Merge works with multiple program instances which will each have their own overlay address space instances. Merge is not suppose to mix addresses and must carefully perform cross-lookups for such things. It's all rather confusing in the code because its never obvious which address factory owns a given overlay space. Hopefully your trace will help isolate.

[ghidra1]

Can this issue be closed?

[mikenawrocki]

Unfortunately I don't think so. I'll try to get more information about the issue soon, thanks!

[ghidra1]

Can this issue be closed?

Sorry, not sure why i asked about closing. We have an open internal ticket to look into/resolve.