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

Renaming struct field does not warn for duplicate names #6444

Open
SlidyBat opened this issue Feb 22, 2025 · 1 comment
Open

Renaming struct field does not warn for duplicate names #6444

SlidyBat opened this issue Feb 22, 2025 · 1 comment
Assignees
Labels
Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround

Comments

@SlidyBat
Copy link

Version and Platform (required):

  • Binary Ninja Version: 4.3.6917-dev Personal (f2344475)
  • OS: macOS
  • OS Version: Sonoma 14.6.1
  • CPU Architecture: M1

Bug Description:
When renaming a struct field, there is no warning/error if a member of the same name already exists. Trying to rename the second field to a different name then results in the wrong field having it's name changed.

Steps To Reproduce:
Please provide all steps required to reproduce the behavior:

  1. Open attached binary and go to main
  2. Click S on rax from first line to create new struct
  3. Click Y on __offset(0x0) on second line and set type to void* buf
  4. Click Y on __offset(0x8) on third line and set type to void* buf. Observe that both fields have same name now
  5. Click N on buf on third line to rename it to buf2. Observe that name of first field changed instead of the correct one.

Expected Behavior:
I think the best behavior would be similar to what happens when you try to name duplicate local variables. A warning that tells you it is a duplicate and lets you confirm / confirm with suffix / cancel.

Alternatively, just making it entirely disallowed and making it an error would also work.

The issue with renaming a field resulting in the renaming of a different field should also be fixed.

Screenshots/Video Recording:

CleanShot.2025-02-23.at.10.22.34.mp4

Binary:

test.zip

Additional Information:
Please add any other context about the problem here.

@xusheng6 xusheng6 added Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround labels Feb 25, 2025
@xusheng6
Copy link
Member

Not allowing two fields to have the same name sounds like a good idea, though I do not know what implications will it bring. I will have the team discuss this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Issue needs changes to the core Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround
Projects
None yet
Development

No branches or pull requests

3 participants