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

[Bug Report] Incorret mask for mcontrol.action #1032

Closed
Phantom1003 opened this issue Jun 16, 2022 · 4 comments
Closed

[Bug Report] Incorret mask for mcontrol.action #1032

Phantom1003 opened this issue Jun 16, 2022 · 4 comments

Comments

@Phantom1003
Copy link
Contributor

Phantom1003 commented Jun 16, 2022

We found the mask for mcontrol.action is 0x3f, while this field is only 4 bits width.
image

action = (triggers::action_t) get_field(val, MCONTROL_ACTION);

#define MCONTROL_ACTION (0x3f<<12)

We triggered this bug by randomly writing data to csr.
If users try to set the sizelo field next to it (although it appears that spike does not yet support), it will cause an illegal action to be saved, and then the abort() will be triggered at line 337 below, causing the simulation to end.

switch (t.action) {
case triggers::ACTION_DEBUG_MODE:
enter_debug_mode(DCSR_CAUSE_HWBP);
break;
case triggers::ACTION_DEBUG_EXCEPTION: {
trap_breakpoint trap(state.v, t.address);
take_trap(trap, pc);
break;
}
default:
abort();

@ProjectDimlight helps reproduce the problem

cc to @timsifive

@Phantom1003
Copy link
Contributor Author

Following is the test case we use, in this program we add a breakpoint to the 0x80000178 and specify the size field is 3.
This is possible because the manual specifies that the fields in mcontrol are WARL, so users may try to write the value they expected(0x200000000003005c), then we try to access 0x80000178, and the log shows that the emulation suddenly stops at 0x80000174.

core   0: 0x0000000080000140 (0x00000593) li      a1, 0
core   0: 0x0000000080000144 (0x7a059073) csrw    tselect, a1
core   0: 0x0000000080000148 (0x00000597) auipc   a1, 0x0
core   0: 0x000000008000014c (0x03058593) addi    a1, a1, 48
core   0: 0x0000000080000150 (0x7a259073) csrw    tdata2, a1
core   0: 0x0000000080000154 (0x7a2025f3) csrr    a1, tdata2
: reg 0 a1  ->  0x0000000080000178
core   0: 0x0000000080000158 (0x0010059b) addiw   a1, zero, 1
core   0: 0x000000008000015c (0x02d59593) slli    a1, a1, 45
core   0: 0x0000000080000160 (0x00358593) addi    a1, a1, 3
core   0: 0x0000000080000164 (0x01059593) slli    a1, a1, 16
core   0: 0x0000000080000168 (0x05c58593) addi    a1, a1, 92
core   0: 0x000000008000016c (0x7a159073) csrw    tdata1, a1
: reg 0 a1 -> 0x200000000003005c    (action was set to 48 here)
core   0: 0x0000000080000170 (0x7a1025f3) csrr    a1, tdata1
core   0: 0x0000000080000174 (0x00100193) li      gp, 1
[exit simulation]

spike-1.zip

@timsifive
Copy link
Collaborator

This is definitely a bug, easily fixed by using the CSR_MCONTROL_ACTION macro instead of MCONTROL_ACTION (which is out-of-date) in triggers.cc. But I've got a bunch of other stuff going on and it will take a while before I get to this.

@scottj97
Copy link
Collaborator

scottj97 commented Dec 5, 2022

I believe this misbehavior was fixed by 7965f25 as part of #1128 (even though encoding.h still has an incorrect value for MCONTROL_ACTION).

@Phantom1003 can you please confirm?

@Phantom1003
Copy link
Contributor Author

After testing, spike no longer exits due to the breakpoint.

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

No branches or pull requests

3 participants