binaryninja-api icon indicating copy to clipboard operation
binaryninja-api copied to clipboard

"Create Function Here" has weird ARM vs Thumb behavior

Open percontation opened this issue 6 years ago • 11 comments

Setup:

  1. I have a small raw binary, it's mostly thumb instructions
  2. I mark some functions as thumb, so Binary Ninja now shows the default architecture as thumb.
  3. There are additional chunks of data I would like to mark as thumb functions.

Behavior:

In the Linear Disassembly view, pressing "P" to mark new functions (on data that hasn't been identified as code) creates non-thumb ARM functions. In the right-click context menu, the "Create Function Here      P" option does not have a submenu that would allow creating a thumb function.

In the Hex Editor view, things are different. The default "P" action does create thumb functions, and the right-click context menu has a submenu that would allow me to pick ARM, Thumb, or whatever else I wanted.

Issue:

The default behavior of the "P" hotkey in Linear Disassembly view isn't what I wanted here. I'm not sure if it's incorrectly using ARM when it should be using thumb, or some heuristics about when to switch between ARM/thumb are just off in this case. Either way, I should have the option to override the default.

Hex Editor view behaves as desired (but obviously, is not convenient to access).

percontation avatar Oct 21 '18 07:10 percontation

Quick illustration of the behavior difference, in case I described it poorly: https://gfycat.com/SpottedNegligibleBlacklemur

Also, noticed this comment: https://github.com/Vector35/binaryninja-api/issues/883#issuecomment-348392365 , but as described above this seems to only be true in Hex Editor view and not Linear Disassembly view.

percontation avatar Oct 21 '18 07:10 percontation

To create a Thumb function you can hit "P" on an odd address.

bpotchik avatar Oct 21 '18 10:10 bpotchik

The P behavior is as designed. As Brian mentioned, the arch is determined based on the address in the case of arm/thumb since that's how the hardware works. But once a default is set, we don't ask again since it's generally the right user experience.

The hex view not accounting for the odd/even doesn't surprise me since we don't have a binary view available there to use to check the address mapping (even if with a flat file the mapping is the same).

The issue about the right click create function being missing in linear view though is not something I can reproduce and would be a problem. Do you have a screen cap of that?

psifertex avatar Oct 21 '18 12:10 psifertex

Ah. Odd address is very easy to remember :)

The issue about right-click create wasn't that it was missing; just that it was different from Hex View and does not give a submenu to select architectures (which made me think I had no way to create a thumb function from linear view, which was my main issue).

Linear view, no submenu to select arch:

Linear view, no submenu to select arch

Hex view has the submenu:

Hex view has the submenu

percontation avatar Oct 22 '18 19:10 percontation

(I retract my suggestion that the linear view needs that arch-choosing submenu, now that I know how to make a thumb function there. Maybe it'd be good for consistency, though? idk.)

percontation avatar Oct 22 '18 19:10 percontation

Ahh, gotcha. Yeah, I see what you mean. Yeah, that's primarily a case of optimizing for the common use case. The situations where you want to make a function with the default architecture requires less clicks/navigation versus the rare situation where you can use a different view. That said, I could go either way on this one. We've got p for the fast use-case, having the linear sweep right-click might match hex might be fine. We'll talk it over internally some. In the meantime i'm going to close this one out since I think the biggest issue was knowing the arm/thumb addressing difference which fixes the primary problem for you.

psifertex avatar Oct 22 '18 20:10 psifertex

We are re-opening this as we believe the current behavior is indeed a bug and we wish to fix it. Here's what we believe should be done:

  • Hitting "p" should always create a function of the default platform.
  • We also believe that the associated platforms (arm/thumb in this case) should be sorted in the "Make Function at this Address" up top

psifertex avatar Jun 07 '23 18:06 psifertex

In my experience with #4244 I was able to create the right Thumb code even not positioning myself on the +1 address. So long as I used the Thumb2 item which was present for me in the create function context menu.

joelreymont avatar Jun 08 '23 05:06 joelreymont

Did pressing p also work for you though? That's the behavior we believe was broken.

psifertex avatar Jun 08 '23 13:06 psifertex

No, it doesn't work, even though the label says P (thumb2), or something like that. What works is drilling down the context menu and selecting thumb2 from there.

joelreymont avatar Jun 08 '23 16:06 joelreymont

The final solution isn't fully decided, but one decision we're leaning towards is 'P' always creating a function with the binaryview's default architecture. That's only useful if the user can easily set the default architecture, so branch set-default-arch-menu has this menu option. Once the setting sticks after saves/loads of bndbs, it will be merged in.

lwerdna avatar Aug 24 '23 04:08 lwerdna

I just got bit by this issue. I'm fine with whatever behavior gets chosen, but in any case the Default (<arch>) message should show the architecture it's actually going to make instructions with. With the current behavior, that would mean showing something like "Default (armv7)" when right clicking on even addresses and "Default (thumb2)" when clicking on odd addresses. Currently, it always shows "Default (thumb2)" for me, which leads to a lot of confusion when I don't get thumb2 instructions.

If this issue is close to being fixed, where the behavior will not change based on the odd and even addresses, then the UI issue gets "fixed" automatically. But if this issue will take some more time, then at least addressing the UI issue would make things less confusing.

gsingh93 avatar Mar 13 '24 17:03 gsingh93