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

User-definable goto location names #987

Closed
Elthial opened this issue Dec 12, 2020 · 3 comments
Closed

User-definable goto location names #987

Elthial opened this issue Dec 12, 2020 · 3 comments

Comments

@Elthial
Copy link

Elthial commented Dec 12, 2020

Add the functionality to rename goto locations names to something more human friendly.

The current names have formats like: "l0FDC_0286", "l0FDC_0289", "l0FDC_028C" which are perfectly servicable but it would be nice to be able to add user defined names so that the decompiled code reads better.

if(v65_861)
goto l0FDC_0529;

VS

if(v65_861)
goto CGA_draw;

@uxmal
Copy link
Owner

uxmal commented Dec 14, 2020

Great suggestion! Now, for the devil in the details.

Today, the label names in Reko have global scope; l00123400 is a global name which can be referred to by any assembler fragment. To make sure the label names don't collide, it's a simple matter of converting the address of a label to text. By making the label names 1:1 with the addresses they correspond to, I easily avoid conflicts.

By allowing the user to arbitrarily change the label names, we're opening up to the potential of conflicts. Suppose I have two procedures fn0010 and fn0030. One of the basic blocks in fn0010 is renamed to error. If a user also renames a block label in fn0030 to error we have a conflict due to the Reko's global scoping rule.

It seems that MASM, the assembler I'm most familiar with, treats statement labels with colons after them as local to the procedure in which they are defined. This is good news for users, as they can now have the label error in as many procedures as they wish. I will therefore make the changes necessary to make this possible with Reko. A new SymbolType, Local, will be introduced. Such symbols refer to basic blocks within the context of a Procedure. Such labels will be saved in the dcproject file.

To create such a label, we need a new context menu on labels with a Rename label command. In an ideal world, this would result in a nice in-place edit of the label in the Reko TextView. However, the TextView doesn't have editing capabilities yet, and I want to hold off making changes to the Windows Forms base since we're presently evaluating moving to another, cross-platform, GUI framework. Will a simple dialog box be satisfactory here?

@Elthial
Copy link
Author

Elthial commented Dec 14, 2020

A simple dialog the same as the "edit declaration" is all that is needed for a MVP.

uxmal added a commit that referenced this issue Jan 21, 2021
uxmal added a commit that referenced this issue Jan 21, 2021
@uxmal
Copy link
Owner

uxmal commented Jan 21, 2021

Right-clicking on a label now shows a new Edit label menu item which when selected displays a dialog where you can enter the name of the label. This is then saved in the project user settings so when you reload the reko project the labels retain their names.

@uxmal uxmal closed this as completed Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants