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

arm64 fork: FORK_REG_LR,FORK_REG_SP should save the func local stack #13664

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

Conversation

lipengfei28
Copy link
Contributor

@lipengfei28 lipengfei28 commented Sep 27, 2024

FORK_REG_LR,FORK_REG_SP should save the func local stack not the last func stack
not the last func stack

Summary

Impact

Testing

@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small labels Sep 27, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 27, 2024

[Experimental Bot, please feedback here]

NuttX PR Requirements Review

Missing Information:

This PR lacks crucial details to assess its compliance with NuttX requirements. Let's break it down:

Summary:

  • Insufficient: "not the last func stack FORK_REG_LR,FORK_REG_SP should save the func local stack not the last func stack" is vague.
    • What is the problem you are solving? Clearly state the bug or why this change is needed.
    • What are FORK_REG_LR and FORK_REG_SP? Provide context for someone unfamiliar with this code section.
    • What is the "func local stack"? Explain the difference between this and the "last func stack".
    • Link to related issue(s): Is there a GitHub issue tracking this bug?

Impact:

  • Entirely missing: You must address each impact category:
    • Feature changes?
    • User impact?
    • Build process changes?
    • Hardware implications?
    • Documentation needs?
    • Security concerns?
    • Compatibility (backward, forward, interoperability)?
    • Other considerations?

Testing:

  • Insufficient: Simply stating "your testing logs here" is not helpful.
    • Provide actual logs: Before and after the change, showing the problem and the solution.
    • Specifics matter: Mention:
      • Host OS and compiler used for building.
      • Target architecture and board configuration.

Overall:

In its current state, this PR is not ready for review. It lacks the necessary information to understand the change, its impact, and how it has been tested. Please address each missing section with clear explanations and supporting details.

@xiaoxiang781216
Copy link
Contributor

@pussuw could you review thtis change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants