Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use AddressSpace equals method for checks #6403

Open
mikenawrocki opened this issue Apr 10, 2024 · 8 comments
Open

Use AddressSpace equals method for checks #6403

mikenawrocki opened this issue Apr 10, 2024 · 8 comments
Assignees
Labels
Status: Internal This is being tracked internally by the Ghidra team

Comments

@mikenawrocki
Copy link
Contributor

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):

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.

@ryanmkurtz ryanmkurtz added the Status: Triage Information is being gathered label Apr 10, 2024
@mikenawrocki
Copy link
Contributor Author

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

@ghidra1
Copy link
Collaborator

ghidra1 commented Apr 10, 2024

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
Copy link
Contributor Author

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
Copy link
Collaborator

ghidra1 commented Apr 10, 2024

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 ghidra1 added Status: Internal This is being tracked internally by the Ghidra team and removed Status: Triage Information is being gathered labels Apr 10, 2024
@ghidra1
Copy link
Collaborator

ghidra1 commented Apr 10, 2024

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
Copy link
Contributor Author

I recall it coming from here specifically:

resultContext.remove(rangeMin, rangeMax, resultReg);

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
Copy link
Contributor Author

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
Copy link
Collaborator

ghidra1 commented Apr 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Internal This is being tracked internally by the Ghidra team
Projects
None yet
Development

No branches or pull requests

3 participants