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

arch/riscv: fix trap sp restore logic #12717

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Jul 17, 2024

Summary

This amends pull #12435 and attempts to fix issue #12635 of PROTECTED build vfork failure since commit e6973c7.

Investigation revealed two separate issues:

  • The optimized dispatch_syscall now requires TP to be prepared, which normally isn't a problem. But for the very first run of the forked child, it was zero. So we need prepare it for the forked child specially before its first run.

  • When returning from exception, the SP restored from stack normally works, As forked child stack is copied from parent, thus restored SP from child stack actually points to the parent stack area! So if child executes ealier that parent reaches the waitpid, parent will crash later due to corrupted stack. Unfortunately this is true with PROTECTED build now.

Impacts

riscv devices

Testing

This fixes `tp` value of forked child in PROTECTED build to support
vfork. Why? the optimized `dispatch_syscall` requires `tp` to hold
the task TCB since commit e6973c7.

Signed-off-by: Yanfeng Liu <[email protected]>
Copy link
Contributor

@anchao anchao left a comment

Choose a reason for hiding this comment

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

LGTM

This fixes stack pointer restore logic to avoid parent stack corruption
by forked child in PROTECTED build.

Signed-off-by: Yanfeng Liu <[email protected]>
Copy link
Member

@lupyuen lupyuen left a comment

Choose a reason for hiding this comment

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

Tested OK on Milk-V Duo S SBC and Ox64 BL808 Emulator. Thanks!

Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

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

Please indicate in the title the affected area for both PR and commits.
This small change will improve the release notes generation

ex
arch/riscv/trap: fix sp restore logic

@yf13 yf13 changed the title arch/riscv: fix issue #12635 arch/riscv: fix trap sp restore logic Jul 17, 2024
@xiaoxiang781216 xiaoxiang781216 merged commit d6c67c5 into apache:master Jul 17, 2024
26 checks passed
@xiaoxiang781216 xiaoxiang781216 linked an issue Jul 17, 2024 that may be closed by this pull request
@yf13 yf13 deleted the 12635 branch August 1, 2024 11:34
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.

riscv vfork failed in PROTECTED build
5 participants