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

Reconsider how the change type dialog handles newly entered types

Open op2786 opened this issue 1 year ago • 2 comments

Currently, when attempting to change a type for a variable or parameter, BN tests if the new type is correct as we type it, before clicking on the Accept button. However, this process can cause lags and make the 'Change Type' dialog unresponsive, especially if an incorrect type is entered.

You can reproduce this by

  1. Clicking on any variable or parameter and pressing Y
  2. Providing a random type, such as "asdasd123"
  3. Pressing Enter a few times (in order to make it more clear)
  4. See how the change type dialog becomes unresponsive

Here is an example GIF:

Recording

I encounter this issue in every RE session, and it often leads to wasted time. I suggest that BN performs type checking only when we press Enter or click on the 'Accept' button. This would eliminate the lag issue and allow for more appropriate error messages to be displayed regarding the error. In current implementation, we need to hover over the "Error parsing specified type" message to see details, which can be a bit difficult to notice.

op2786 avatar Jan 25 '24 14:01 op2786

So there are two different issues here:

  1. Parsing types takes a while (especially if it's invalid)
  2. The UI waits on the background thread

I can add a setting to disable the background parsing, which will speed up things a bit. But the actual parse will still be slow. Looks like there are performance issues trying to resolve type names.

CouleeApps avatar Apr 04 '24 19:04 CouleeApps

Build 4.1.5071-dev will have two changes to address this:

  1. You can disable the UI background type checking via the new Setting ui.types.checkParse
  2. Parts of the parsing setup process were sped up a bit, though having a ton of types will still cause it to be slower

CouleeApps avatar Apr 04 '24 20:04 CouleeApps