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

Test arch arm thread swap minor fix #19310

Conversation

ioannisg
Copy link
Member

Fixes a bug in erroneous zassert expression in arm thread swap test suite

Fixes #18999

@ioannisg ioannisg added bug The issue is a bug, or the PR is fixing a bug area: ARM ARM (32-bit) Architecture labels Sep 20, 2019
@ioannisg ioannisg requested a review from pabigot September 20, 2019 23:00
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Sep 20, 2019
@ioannisg ioannisg added this to the v2.1.0 milestone Sep 21, 2019
Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. Please fix the preprocessor whitespace issue. I would go with the code change, but there may be subtleties I'm not seeing so that's just a suggestion.

We use inline assembly to store the return value of _swap(..)
function directly into r0 (in order to ensure that r4-r11
registers are not touched at this point). But we need to store
the r0 into some global memory, to retain the value until we
check it later in an assert expression, otherwise the
compiler may overwrite r0 in subsequent instructions.

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
The commit fixes two assert expressions in the test,
which evaluate the return value of _swap(.) function
and the value of the thread's swap return variable.

Signed-off-by: Ioannis Glaropoulos <Ioannis.Glaropoulos@nordicsemi.no>
@ioannisg ioannisg force-pushed the test_arch_arm_thread_swap_minor_fix branch from cd7e38e to 45bd788 Compare September 21, 2019 13:36
Copy link
Member Author

@ioannisg ioannisg left a comment

Choose a reason for hiding this comment

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

@pabigot thanks for your comments, I applied your feedback.
Let me know if this plays OK over #18991

@pabigot
Copy link
Collaborator

pabigot commented Sep 21, 2019

The original (and patched) version passed under #18991 so this should be good. Thanks.

@ioannisg ioannisg merged commit 44f3b79 into zephyrproject-rtos:master Sep 22, 2019
@backporting
Copy link

backporting bot commented Sep 22, 2019

The backport to v1.14-branch failed:

Commits ["45bd788059e1225571909911772bdb6de95f508e","7f47cf7bc055df69e4f113f6fa83d3a9d2898c6c"] could not be cherry-picked on top of v1.14-branch

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport v1.14-branch
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick 45bd788059e1225571909911772bdb6de95f508e 7f47cf7bc055df69e4f113f6fa83d3a9d2898c6c
# Create a new branch with these backported commits.
git checkout -b backport-19310-to-v1.14-branch
# Push it to GitHub.
git push --set-upstream origin backport-19310-to-v1.14-branch
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is v1.14-branch and the compare/head branch is backport-19310-to-v1.14-branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assignment in assert in test of arm_thread_arch causes build failures
3 participants