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

fix incorrect register number in npc #11

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

Conversation

FurryAcetylCoA
Copy link

Follow up to f198eb6


However, I personally recommend keeping this incorrect register number in both npc and nemu 's trap.S and leave it as a part of "3-2 恢复上下文" (maybe add a hint)
This is because locating this bug requires a decent amount of effort, and students are compelled to meticulously check the structure of the frame prepared by trap.S.
It presents a great opportunity to enhance students' ability to use debugging tools. During this debugging session, students mush figure out exactly what is stored and loaded during content switch and why such a tiny difference will leads to the corruption of the frame ( for example: in yield-os, it stops at first B and not switching back to A). This will also enforce " 我乱改一通, 居然过了, 嘿嘿嘿", given that they will no pass (at least in RISC-V).

By the way this bug can be described as "the disparity in add/sub of sp, which leading the stack pointer drift 4 bytes every times a context switch occours in __am_asm_trap. This discrepancy is the result of a diffence between the value of CONTEXT_SIZE and sizeof(Context)). CONTEXT_SIZE is defined as (NR_REGS + 3 + 1) * XLEN. NR_REGS is for gpr, and 3 is for csr, obviously. Then WTF (f for friendly) is that 1? Anyway, I have to expand the Context struct to match that."

With that being said. This pitfall only exist in RISC-V. Which is a little bit unfair. Or maybe just forget about what I just said...


省流助手: 我觉得这个bug可以留着,因为这是个不错的练习。

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

Successfully merging this pull request may close these issues.

1 participant