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

v850 support for getting main offset #2380

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

PeiweiHu
Copy link
Contributor

@PeiweiHu PeiweiHu commented Mar 4, 2022

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

Fix #2316. This pr adds support for v850 while getting main offset.

...

Test plan

If possible, I hope more v850 samples can be provided for the test (I searched but found only a few).

...

Closing issues

...

@PeiweiHu PeiweiHu marked this pull request as draft March 4, 2022 07:03
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Please add a testcase.

librz/bin/format/elf/elf_info.c Outdated Show resolved Hide resolved
@wargio
Copy link
Member

wargio commented Mar 4, 2022

Can you also add a test? (open a PR on rizin bin repo if needed)

@PeiweiHu
Copy link
Contributor Author

PeiweiHu commented Mar 7, 2022

Explanation for changing test/db/analysis/v850:

I ran aaa; afl on other samples, main is displayed instead of sym.main.

@wargio
Copy link
Member

wargio commented Mar 7, 2022

i think we need to test this on a bin without symbols.

@PeiweiHu
Copy link
Contributor Author

@PeiweiHu could you please rebase and update the PR?

Sorry for the late response. Since this pr is not thoroughly tested, I will refine and reopen this in June (after some of my deadlines) if nobody solves it.

@PeiweiHu PeiweiHu closed this May 14, 2022
@XVilka
Copy link
Member

XVilka commented Jan 14, 2024

@PeiweiHu could you please reopen/finish this one again? It would help the ongoing V850 RzIL conversion: #4103

For creating testcases these toolchains could be used:

cc @imbillow

@XVilka XVilka added this to the 0.7.0 milestone Jan 14, 2024
@PeiweiHu
Copy link
Contributor Author

Ok, I will try to finish this when I am free. cc @XVilka

Copy link
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

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

nice

@PeiweiHu PeiweiHu marked this pull request as ready for review January 15, 2024 15:39
@PeiweiHu PeiweiHu requested a review from thestr4ng3r as a code owner January 15, 2024 15:39
@XVilka XVilka merged commit 90c2e5b into rizinorg:dev Jan 16, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rizin cannot find main or sym.main function in V850 ELF
4 participants