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

uplifting h8300 instructions to RzIL #4375

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

ashiskumarnaik
Copy link

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

RzIL upliftment for H8300

...

...

for #2080

...

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.

  1. Requires a test for RzIL, see how it's done, in test/db/asm/h8300
  2. Windows linking error (you need to update Meson files
  Creating library binrz/rz-run/rz-run.lib and object binrz/rz-run/rz-run.exp
rz_arch.lib(p_arch_h8300.c.obj) : error LNK2001: unresolved external symbol rz_h8300_il_config
rz_arch.lib(p_arch_h8300.c.obj) : error LNK2019: unresolved external symbol rz_h8300_il_opcode referenced in function h8300_op
binrz/rz-run/rz-run.exe : fatal error LNK1120: 2 unresolved externals
[1685/1953] Compiling C object binrz/rz-asm/rz-asm.exe.p/rz-asm.c.obj
ninja: build stopped: subcommand failed.
Command exited with code 1

@@ -0,0 +1,491 @@
//
// Created by Ashis Kumar Naik on 21/03/24.
Copy link
Member

Choose a reason for hiding this comment

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

Should be SPDX instead

Copy link
Author

Choose a reason for hiding this comment

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

updated.

@@ -0,0 +1,14 @@
//
// Created by Ashis Kumar Naik on 21/03/24.
Copy link
Member

Choose a reason for hiding this comment

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

SPDX

Copy link
Author

Choose a reason for hiding this comment

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

updated.

Signed-off-by: Ashis Kumar Naik <[email protected]>
@ashiskumarnaik ashiskumarnaik marked this pull request as ready for review March 23, 2024 09:00
@@ -12,6 +12,8 @@

#include <h8300/h8300_disas.h>

#include "isa/h8300/h8300_il.h"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include "isa/h8300/h8300_il.h"
#include <h8300/h8300_il.h>

@@ -231,6 +231,7 @@ arch_isa_sources = [
'isa/sh/disassembler.c',
'isa/sh/lookup.c',
'isa/sh/sh_il.c',
'isa/h8300/h8300_il.c',
Copy link
Member

Choose a reason for hiding this comment

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

move this under line 159.

@XVilka
Copy link
Member

XVilka commented Mar 25, 2024

Don't forget to rebase when you do your next change.

@XVilka XVilka mentioned this pull request Mar 29, 2024
38 tasks
@XVilka
Copy link
Member

XVilka commented Apr 10, 2024

@ashiskumarnaik have you had any chance to fix this PR?

@ashiskumarnaik
Copy link
Author

@XVilka Hi , I am still working on it.

@XVilka
Copy link
Member

XVilka commented Apr 21, 2024

Any updates?

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.

It would require a "h8300" file with RzIL tests in this directoy as well: https://github.com/rizinorg/rizin/tree/dev/test/db/rzil

Including the "emulateme" type of test, where RzIL is used to emulate the part of the binary (see already existing tests in this directory).

For the instruction-level tests you will need to extend the https://github.com/rizinorg/rizin/blob/dev/test/db/asm/h8300 file with RzIL strings (see other files in that directory as an example)

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.

None yet

4 participants