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

Add HPPA(PA-RISC) architecture #2265

Merged
merged 37 commits into from
Mar 26, 2024
Merged

Conversation

R33v0LT
Copy link
Contributor

@R33v0LT R33v0LT commented Feb 3, 2024

This PR adds HPPA 1.1/2.0 support
A checklist of necessary steps before changing the state from the draft:

  • HPPA 1.1 instructions support
  • HPPA 2.0 instructions support
  • Support instruction details
  • cstests for all versions
  • Python bindings/tests

cstool/cstool_hppa.c Outdated Show resolved Hide resolved
tests/test_hppa.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Left some suggestions. Good work!

MCInst.h Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPAInstPrinter.c Outdated Show resolved Hide resolved
@R33v0LT R33v0LT marked this pull request as ready for review March 4, 2024 18:17
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Good job!
I added many comments, though some are duplicates and none of them should be much work.
Just many tiny things.

Please, when you have resolved all of them, we need to do at least two more things:

  1. Add HPPA to the .github/labeler.yml
  2. Run clang-format on all .c and .h files. The formatting is a little messed up. There is a .clang-format file in the repo root. Please use this one.
  3. Fuzzing. Every byte combination from 0x0-0xffffffff should work without segfault. Please, apply the following patch:
    diff --git a/cstool/cstool.c b/cstool/cstool.c
    index 2240ce132..4b6b62887 100644
    --- a/cstool/cstool.c
    +++ b/cstool/cstool.c
    @@ -607,6 +607,19 @@ int main(int argc, char **argv)
     		cs_option(handle, CS_OPT_DETAIL, CS_OPT_DETAIL_REAL);
     	}
     
    +	uint32_t bytes = 0xffffffff;
    +	while (address == 0xffffffff) {
    +		printf("\r0x%08x\t\t", bytes);
    +		fflush(stdout);
    +		count = cs_disasm(handle, (uint8_t*)&bytes, 4, address, 0, &insn);
    +		if (insn && insn->detail)
    +			free(insn->detail);
    +		free(insn);
    +		bytes++;
    +		if (bytes == 0xffffffff)
    +			return 0;
    +	}
    +
     	count = cs_disasm(handle, assembly, size, address, 0, &insn);
     	if (count > 0) {
     		size_t i;
    and test with and without details:
    ./cstool -d hppa11 0 0xffffffff
    ./cstool hppa11 0 0xffffffff
    ./cstool -d hppa20w 0 0xffffffff
    ./cstool hppa20w 0 0xffffffff
    

arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
@XVilka XVilka mentioned this pull request Mar 15, 2024
10 tasks
@Rot127
Copy link
Collaborator

Rot127 commented Mar 18, 2024

@R33v0LT Please request my review when you're done. Checking the changes in total is easier.

@Rot127
Copy link
Collaborator

Rot127 commented Mar 20, 2024

Almost forgot, please add the HPPA files to the .github/labeler.yml. Label name is: HPPA

@Rot127 Rot127 added this to the v6 milestone Mar 20, 2024
@Rot127 Rot127 added the HPPA Arch label Mar 22, 2024
@R33v0LT R33v0LT requested a review from Rot127 March 22, 2024 15:38
@R33v0LT
Copy link
Contributor Author

R33v0LT commented Mar 22, 2024

Good job! I added many comments, though some are duplicates and non of them should be much work. Just many tiny things.

Please, when you have resolved all of them, we need to do at least two more things:

  1. Add HPPA to the .github/labeler.yml

  2. Run clang-format on all .c and .h files. The formatting is a little messed up. There is a .clang-format file in the repo root. Please use this one.

  3. Fuzzing. Every byte combination from 0x0-0xffffffff should work without segfault. Please, apply the following patch:

    diff --git a/cstool/cstool.c b/cstool/cstool.c
    index 2240ce132..4b6b62887 100644
    --- a/cstool/cstool.c
    +++ b/cstool/cstool.c
    @@ -607,6 +607,19 @@ int main(int argc, char **argv)
     		cs_option(handle, CS_OPT_DETAIL, CS_OPT_DETAIL_REAL);
     	}
     
    +	uint32_t bytes = 0xffffffff;
    +	while (address == 0xffffffff) {
    +		printf("\r0x%08x\t\t", bytes);
    +		fflush(stdout);
    +		count = cs_disasm(handle, (uint8_t*)&bytes, 4, address, 0, &insn);
    +		if (insn && insn->detail)
    +			free(insn->detail);
    +		free(insn);
    +		bytes++;
    +		if (bytes == 0xffffffff)
    +			return 0;
    +	}
    +
     	count = cs_disasm(handle, assembly, size, address, 0, &insn);
     	if (count > 0) {
     		size_t i;

    and test with and without details:

    ./cstool -d hppa11 0 0xffffffff
    ./cstool hppa11 0 0xffffffff
    ./cstool -d hppa20w 0 0xffffffff
    ./cstool hppa20w 0 0xffffffff
    

Done without faults

Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Found a few more. Please fix them as well.

MathExtras.h Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
arch/HPPA/HPPADisassembler.c Outdated Show resolved Hide resolved
include/capstone/hppa.h Show resolved Hide resolved
include/capstone/hppa.h Outdated Show resolved Hide resolved
@R33v0LT R33v0LT requested a review from Rot127 March 24, 2024 18:38
Copy link
Collaborator

@Rot127 Rot127 left a comment

Choose a reason for hiding this comment

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

Very nice!
@kabeor Please take a look.

Copy link
Member

@kabeor kabeor left a comment

Choose a reason for hiding this comment

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

Looks really good, thank you again! Merged.

@kabeor kabeor merged commit 9daa1ff into capstone-engine:next Mar 26, 2024
12 checks passed
@Rot127 Rot127 mentioned this pull request May 15, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants