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

librz/core: Fix disassembly (e.g. pd) with unmapped gaps #4932

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

thestr4ng3r
Copy link
Member

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 every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).

Detailed description

Regression from 3b7f273:
rz_io_nread_at stops at unmapped regions, but disassembly being a
user-visible display needs to show gaps as 0xff and mapped regions after
with their contents again instead of just breaking and showing io->Oxff.
The exact meaningd of rz_io_nread_at and rz_io_read_at_mapped have also
been clarified in documentation comments and tests.

Test plan

$ rz -a arm -b 64
[0x00000000]> o malloc://1024 0x10
[0x00000000]> pd 4 @ 0x10
            0x00000010      00000000       udf   0
            0x00000014      00000000       udf   0
            0x00000018      00000000       udf   0
            0x0000001c      00000000       udf   0
[0x00000000]> pd 4 @ 0xc
            0x0000000c      ffffffff       invalid
            0x00000010      ffffffff       invalid
            0x00000014      ffffffff       invalid
            0x00000018      ffffffff       invalid

Notice how in the second pd, all bytes starting at 0x00000010 are read incorrectly.

After:

$ rz -a arm -b 64
[0x00000000]> o malloc://1024 0x10
[0x00000000]> pd 4 @ 0x10
            0x00000010      00000000       udf   0
            0x00000014      00000000       udf   0
            0x00000018      00000000       udf   0
            0x0000001c      00000000       udf   0
[0x00000000]> pd 4 @ 0xc
            0x0000000c      ffffffff       invalid
            0x00000010      00000000       udf   0
            0x00000014      00000000       udf   0
            0x00000018      00000000       udf   0

Regression from 3b7f273:
rz_io_nread_at stops at unmapped regions, but disassembly being a
user-visible display needs to show gaps as 0xff and mapped regions after
with their contents again instead of just breaking and showing io->Oxff.
The exact meaningd of rz_io_nread_at and rz_io_read_at_mapped have also
been clarified in documentation comments and tests.
Copy link
Member

@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.

Just this one nitpick. Otherwise feel free to merge.

@@ -3152,15 +3152,16 @@ static bool core_disassembly(RzCore *core, int n_bytes, int n_instrs, RzCmdState
.cbytes = cbytes,
};

ut8 *buf = malloc(n_bytes + 1);
ut8 *buf = calloc(1, n_bytes + 1);
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
ut8 *buf = calloc(1, n_bytes + 1);
ut8 *buf = RZ_NEWS0(ut8, n_bytes + 1);

More a fan of RZ_NEWS0.
Mostly because we encourage new comers to use our macros and not calloc

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.

3 participants