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

Refactored Decompiler Widget and R2Dec Plugin to use RAnnotatedCode #2227

Conversation

NirmalManoj
Copy link
Member

@NirmalManoj NirmalManoj commented Jun 2, 2020

Your checklist for this pull request

Detailed description
[Update: This was merged to master by PR #2241]

This PR refactors decompiler widget and r2dec cutter plugin to use RAnnotatedCode instead of the custom AnnotatedCode we have been using. Using RAnnotatedCode will enable us to have more data about the decompiled code that is essential for a good decompiler.

The following are the major changes I have made.

  1. Refactored R2Dec::decompileAt() to emit RAnnotatedCode*.
  2. Deleted structures AnnotatedCode, CodeAnnotation, and functions that made to handle these structures.
  3. Used a smart pointer to represent code in DecompilerWidget
    std::unique_ptr<RAnnotatedCode> code;
  4. Refactored slot decompilationFinished(AnnotatedCode code) to decompilationFinished(RAnnotatedCode *code)
  5. Made two static functions static ut64 offsetForPosition(RAnnotatedCode *codeDecompiled, size_t pos) and static size_t positionForOffset(RAnnotatedCode *codeDecompiled, ut64 offset). These are the corresponding functions that were deleted from AnnotatedCode. Made required changes in all functions that use these two functions.

Test plan (required)

Use only R2Dec decompiler for these tests, other decompilers are outside the scope of this PR.

  1. Fetch the PR and compile it.
  2. Open the decompiler widget and make sure that the correct decompiled code is being shown while you select offsets from functions and other widgets.
  3. Make sure that the decompiler widget is seeking the correct address while clicking on functions(and other offsets) in the decompiler widget.
  4. Make sure that invalid offsets are not causing Segmentation Fault.

Notes
Merge all these three PRs together to ensure smooth functioning of all decompilers.
Refactored decompiler widget and R2Dec plugin: Cutter #2227
Refactored R2Ghidra plugin that emits RAnnotatedCode: R2Ghidra-dec #112
Refactored retdec plugin that emits RAnnotatedCode: Florian's fork #1, after this PR is verified, I will send it to retdec's repo.

src/common/Decompiler.h Show resolved Hide resolved
src/widgets/DecompilerWidget.cpp Outdated Show resolved Hide resolved
src/widgets/DecompilerWidget.cpp Outdated Show resolved Hide resolved
this->code = code;
if (code.code.isEmpty()) {
this->code = std::unique_ptr<RAnnotatedCode>(codeDecompiled);
QString codeString = QString::fromUtf8(this->code->code);
Copy link
Member

@karliss karliss Jun 2, 2020

Choose a reason for hiding this comment

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

If the assumption is that code->code is utf8. Then the offsets within RAnnotatedCode can't be used directly without mapping them to whatever Qt uses.

Copy link
Member

@karliss karliss Jun 2, 2020

Choose a reason for hiding this comment

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

The mapping doesn't need to be implemented withing this PR. But you have to take into account that such conversion will be necessary.

@NirmalManoj NirmalManoj requested a review from karliss Jun 4, 2020
Copy link
Member

@karliss karliss left a comment

Few minor comments otherwise looks good enough.

src/common/Decompiler.h Outdated Show resolved Hide resolved
src/common/Decompiler.cpp Outdated Show resolved Hide resolved
src/common/Decompiler.cpp Outdated Show resolved Hide resolved
karliss
karliss approved these changes Jun 8, 2020
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

Successfully merging this pull request may close these issues.

None yet

3 participants