Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Check instruction misalignment for RV32C in the op_branch function #14

Merged
merged 1 commit into from
Jan 24, 2022

Conversation

dougpuob
Copy link
Contributor

@dougpuob dougpuob commented Jan 24, 2022

I encountered a signal SIGABRT error.

000104b2
00027798  _dl_aux_init
0002779a
0002779e
00000000
rv32emu: io.c:76: memory_read_ifetch: Assertion `c' failed.
fish: Job 1, 'build/rv32emu ~/petzone/llvm/da…' terminated by signal SIGABRT (Abort)

Why:
The conditions are equal making the beqz takes action to jump to the PC, which points to 0x2798a. Then the instruction misalignment checking mechanism makes the PC was reset to zero in the op_branch function. This makes next instruction fetch got problem in memory_read_ifetch() function.

00027798 <_dl_aux_init>:
   27798:	411c         lw	    a5,0(a0)
   2779a:	eca1a223     sw     a0,-316(gp) # 6f218 <_dl_auxv>
   2779e:	1e078663     beqz   a5,2798a <_dl_aux_init+0x1f2> -----+
   ...                                                                 |
   2798a:	8082         ret    <----------------------------------+ 
                                     The `beqz` should jump to here (0x2798a), but the program counter PC
                                     was reset to zero, making the `beqz` instruction cannot be executed completely.
   ^^^^^

How:
According to the chapter, "1.2 Instruction Length Encoding" in the specification (riscv-spec-v2.2.pdf), the least significant two bits of PC is possible to be 00b, 01b, and 10b with RV32C. The gcc and clang implemented the CB format with 01b and other parts in this project did so, so I add a corresponding check too.

@dougpuob dougpuob marked this pull request as ready for review January 24, 2022 06:11
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Improve the git commit message to reflect what you found.

@dougpuob dougpuob force-pushed the fix-inst-misalign-for-op_branch branch 2 times, most recently from 91b7c29 to c38b0b1 Compare January 24, 2022 06:43
@dougpuob
Copy link
Contributor Author

Improve the git commit message to reflect what you found.

Hi @jserv,
Thank you for reminding me. I have updated the commit message.

@jserv
Copy link
Contributor

jserv commented Jan 24, 2022

Hi @jserv, Thank you for reminding me. I have updated the commit message.

Please check this carefully: https://cbea.ms/git-commit/

  • Do not end the subject line with a period

In addition, avoid using markdown syntax within git commit message(s) since the later is intended to be read by human rather than machines.

@dougpuob dougpuob force-pushed the fix-inst-misalign-for-op_branch branch from c38b0b1 to e02c1f1 Compare January 24, 2022 13:06
I encountered a signal SIGABRT error.

000104b2
00027798  _dl_aux_init
0002779a
0002779e
00000000
rv32emu: io.c:76: memory_read_ifetch: Assertion `c' failed.
fish: Job 1, 'build/rv32emu ~/petzone/…' terminated by signal SIGABRT (Abort)

Why:
The conditions are equal making the beqz takes action to jump to the PC, which
points to 0x2798a. Then the instruction misalignment checking mechanism makes
the PC was reset to zero in the op_branch function. This makes next instruction
fetch got problem in memory_read_ifetch() function.

00027798 <_dl_aux_init>:
   27798:  411c         lw     a5,0(a0)
   2779a:  eca1a223     sw     a0,-316(gp) # 6f218 <_dl_auxv>
   2779e:  1e078663     beqz   a5,2798a <_dl_aux_init+0x1f2>
   ...
   2798a:  8082         ret

The `beqz` should jump to here (0x2798a), but the program counter PC
was reset to zero, making the `beqz` instruction cannot be executed completely.

How:
According to the chapter, "1.2 Instruction Length Encoding" in the specification
(riscv-spec-v2.2.pdf), the least significant two bits of PC is possible to be
00b, 01b, and 10b with RV32C. The gcc and clang implemented the CB format with
01b and other parts in this project did so, so I add a corresponding check too.
@dougpuob dougpuob force-pushed the fix-inst-misalign-for-op_branch branch from e02c1f1 to 1a7b2d9 Compare January 24, 2022 13:14
@dougpuob
Copy link
Contributor Author

Hi @jserv, Thank you for reminding me. I have updated the commit message.

Please check this carefully: https://cbea.ms/git-commit/

  • Do not end the subject line with a period

In addition, avoid using markdown syntax within git commit message(s) since the later is intended to be read by human rather than machines.

@jserv, Thank you for the suggestions. I have updated to improve the following:

  • Wrapped body less than 80 chars,
  • Removed the period from the subject line,
  • Removed the markdown syntax.

@jserv jserv changed the title Check instruction misalignment for RV32C in the op_branch function. Check instruction misalignment for RV32C in the op_branch function Jan 24, 2022
@jserv jserv merged commit 709389e into sysprog21:master Jan 24, 2022
@jserv
Copy link
Contributor

jserv commented Jan 24, 2022

Thank @dougpuob for contributing!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants