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 documentation for the new decompiler #2394

Merged
merged 17 commits into from Aug 31, 2020

Conversation

NirmalManoj
Copy link
Member

@NirmalManoj NirmalManoj commented Aug 17, 2020

Your checklist for this pull request

Detailed description
This is the documentation for the new context menu for the decompiler implemented by PR #2391 .

Test plan (required)

  1. Read all the documentation. Think if anything can be improved.
  2. Make sure I didn't miss out anything.

@NirmalManoj NirmalManoj self-assigned this Aug 17, 2020
@NirmalManoj NirmalManoj changed the base branch from decompiler-refactoring to master Aug 20, 2020
@NirmalManoj NirmalManoj requested review from ITAYC0HEN and karliss Aug 20, 2020
@NirmalManoj NirmalManoj marked this pull request as ready for review Aug 20, 2020
@ITAYC0HEN ITAYC0HEN changed the title User documentation for the new decompiler {Do not merge} User documentation for the new decompiler Aug 20, 2020
@ITAYC0HEN
Copy link
Member

ITAYC0HEN commented Aug 20, 2020

Didn't review yet.

But I expect a real user documentation and not only menu reference. Means, a dedicated "r2book"-like page for the decompiler widget. Yes, the decompilers are different from each other and we only provide the interface, and still.

See for example Ghidra's. Partial screenshot:
image

Full source:
https://github.com/NationalSecurityAgency/ghidra/blob/master/Ghidra/Features/Decompiler/src/main/help/help/topics/DecompilePlugin/Decompiler.htm

and there are few nice things in IDA's
https://www.hex-rays.com/products/decompiler/compare/compare_vs_disassembly/
https://www.hex-rays.com/products/decompiler/manual/index.shtml

I also expect to refer the readers to available decompilers -> https://github.com/radareorg/cutter-plugins#decompilers

Also to mention that r2ghidra is the official plugin maintained but the devs of cutter and r2.
I also would like to see some tips for how to create a new decompiler. I'd like to see the information for devs about the structure of the annotations (maybe better to be put in a mutual place with r2).

Also to show how it can work with debugger and list common issue s(oh boy there are...), how it can be synced with the disassembly, and more and more. I want the reader to go to a journey in a book to understand the decompiler in cutter.

As it is now, I think it is far from enough. It is simply a context menu reference...

@NirmalManoj
Copy link
Member Author

NirmalManoj commented Aug 20, 2020

@ITAYC0HEN Where do you want such a page that introduces the Cutter's decompiler to appear?

also would like to see some tips for how to create a new decompiler. I'd like to see the information for devs about the structure of the annotations

I can explain the structure of the annotations. But what did you actually mean by the second(Edit: first) sentence?

@ITAYC0HEN
Copy link
Member

ITAYC0HEN commented Aug 21, 2020

Most likely docs/user-docs/widgets/decompiler (see https://github.com/radareorg/cutter/pull/2333/files#diff-9f0df8306ff83684aadf4515c6751ded). I am also putting on the table the option of docs/user-docs/features/decompiler or anything similar that you guys will suggest. I can see pros and cons for bot widgets/ and features/.

Regarding the second question. So say you have your own decompiler, called NirmalDecompiler. It is written in C++ and you want it to add support for this decompiler in both radare2 and Cutter. What steps should you take?

This part will also describe the current issues and limitations of the lack of decompielr interface from the radare2 side, and explain why at all the plugin for Cutter is needed. It will also describe how and why things are working as they are. Or in other words, this is another book chapter about adding a decompiler for Cutter (and naturally, r2). Currently this knowledge isn't shared anywhere and a developer need to copy from other projects.

Yes, I know we need a better interface for decompilers in radare2, this will remove the need for the Cutter part in the decompiler, but currently this is not the case. And as you currently hold all this knowledge in you head, we need to see it on the docs.

I know @karliss might have different opinions so let's hear it before you move forward for the dev documentation (user docs should proceed as discussed)

@NirmalManoj
Copy link
Member Author

NirmalManoj commented Aug 24, 2020

I know @karliss might have different opinions so let's hear it before you move forward for the dev documentation (user docs should proceed as discussed)

@karliss Do you have different opinions on the documentation for developers?

Copy link
Member

@ITAYC0HEN ITAYC0HEN left a comment

Very nice Nirmal! :)

I will have more review comments, especially about features.rst

docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
----------------------------------------
**Description:** Copy the instruction address mapped to the part of code under the cursor.

**Steps:** Right-click on a location and choose ``Copy instruction address(<address>)``
Copy link
Member

@ITAYC0HEN ITAYC0HEN Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

location?

Copy link
Member Author

@NirmalManoj NirmalManoj Aug 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased to Copy the address of the instruction mapped to the part of code under the cursor.

docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
----------------------------------------
**Description:** Show the code under cursor in another opened widget, or open a new one. If a non-decompiler widget is chosen, the address mapped to the portion of code under the cursor will be opened in that widget.

**Steps:** Right-click on an item and choose the ``Show in`` sub-menu.
Copy link
Member

@ITAYC0HEN ITAYC0HEN Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you dont choose the show-in, but a widget from inside this menu

Copy link
Member Author

@NirmalManoj NirmalManoj Aug 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased to Right-click on an item and go to the :menuselection:`Show in` submenu. You can choose a widget or open a new widget from here.

docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
----------------------------------------
**Description:** Open the advanced breakpoint dialog. This dialog lets you define not only a regular breakpoint in this address, but also a Hardware breakpoint, a conditional breakpoint, and more.

**Steps:** Right-click on an instruction and choose ``Breakpoint -> Advanced breakpoint``. If multiple breakpoints are present in the line, you will be able choose the breakpoint you want to edit from the ``Edit breakpoint`` submenu.
Copy link
Member

@ITAYC0HEN ITAYC0HEN Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@NirmalManoj NirmalManoj Aug 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed as suggested.

docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
Copy link
Member

@Surendrajat Surendrajat left a comment

Hail Grammarly :)

docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
docs/source/user-docs/menus/decompiler-context-menu.rst Outdated Show resolved Hide resolved
@karliss
Copy link
Member

karliss commented Aug 29, 2020

@karliss Do you have different opinions on the documentation for developers?

I have no objections against having a page in developer docs describing high level structure of decompiler in cutter and R2. What are the components involved, what are their main responsibilities and how they communicate with each other.

It shouldn't duplicate the API documentation by trying to describe each method or each annotation. It doesn't need to be a tutorial for people who can barely program.

To summarize here is what I would like to see for developer docs:

  • Document the Decompiler interface using doxygen
  • Page in developer docs describing high level structure of decompiler in Cutter
    • What are the components involved (r2 plugin, r2, Cutter, Cutter plugin[cutter plugin impl, Cutter Decompiler impl])
    • Role of each component. Described using human language instead of literal translation code into English. Description like "This module calls this function, then it calls that function using the result function and then it calls one more function" is in my opinion worse than no documentation and reading the code directly. At least when reading the code it's up to date.
    • How the components communicate with each other, at high level, details should be in API docs
    • Checklist of what needs to be implemented (Cutter Plugin,Cutter Decompiler, r2 plugin optional but recommended)

@NirmalManoj
Copy link
Member Author

NirmalManoj commented Aug 29, 2020

@ITAYC0HEN I am not sure if I could make developer documentation in the next two days like Karliss wants. So I'm thinking of doing it after my project in another PR. User documentation is important, so I don't want developer doc to block this PR from getting merged.

@ITAYC0HEN ITAYC0HEN added the Decompiler Issues and feature requests related to the decompiler in Cutter label Aug 30, 2020
@ITAYC0HEN
Copy link
Member

ITAYC0HEN commented Aug 30, 2020

@NirmalManoj this is fine :) I prefer you relaxed and focused on the things you must do, rather then submitting ahlf-baked dev docs. So, no problems - let's keep dev docs for a following PR :)

@NirmalManoj NirmalManoj merged commit 27dcefd into rizinorg:master Aug 31, 2020
9 checks passed
@NirmalManoj NirmalManoj moved this from In progress to Done in Improving Decompiler Widget (GSoC) Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Decompiler Issues and feature requests related to the decompiler in Cutter Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants