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

Invalid thread indexes out of userspace generation #43618

Closed
andyross opened this issue Mar 9, 2022 · 1 comment · Fixed by #43622
Closed

Invalid thread indexes out of userspace generation #43618

andyross opened this issue Mar 9, 2022 · 1 comment · Fixed by #43622
Assignees
Labels
area: Userspace Userspace bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug

Comments

@andyross
Copy link
Contributor

andyross commented Mar 9, 2022

This was discovered during the merge and quick revert of #43547 but it actually seems to be a bug in the userspace build generators. You can see it by applying this to HEAD (as of commit 9fe466f) and then building tests/kernel/semaphore/semaphore on x86_64:

diff --git a/subsys/testsuite/ztest/src/ztest.c b/subsys/testsuite/ztest/src/ztest.c
index 58c440b5fe..8686a104d4 100644
--- a/subsys/testsuite/ztest/src/ztest.c
+++ b/subsys/testsuite/ztest/src/ztest.c
@@ -97,7 +97,7 @@ static int cleanup_test(struct unit_test *test)
 #endif
 #define CPUHOLD_STACK_SZ (512 + CONFIG_TEST_EXTRA_STACK_SIZE)
 
-static struct k_thread cpuhold_threads[NUM_CPUHOLD];
+static struct k_thread cpuhold_threads[NUM_CPUHOLD + 1];
 K_KERNEL_STACK_ARRAY_DEFINE(cpuhold_stacks, NUM_CPUHOLD, CPUHOLD_STACK_SZ);
 static struct k_sem cpuhold_sem;
 volatile int cpuhold_active;

It then fails on the very first test case with:

SeaBIOS (version zephyr-v1.0.0-0-g31d4e0e-dirty-20200714_234759-fv-az50-zephyr)
Booting from ROM..*** Booting Zephyr OS build zephyr-v3.0.0-917-g9fe466f4b051  ***
Running test suite test_semaphore
===================================================================
START - test_k_sem_define
E: thread 0x11fa20 (-1) does not have permission on k_sem 0x17b238
E: permission bitmap
E: 00 00                   |..      
E: syscall z_vrfy_k_sem_count_get failed check: access denied
E: Bad system call from RIP 0x1001cf
E: >>> ZEPHYR FATAL ERROR 3: Kernel oops on CPU 0
E: Current thread: 0x11fa20 (test_k_sem_define)
Caught system error -- reason 3 0
Fatal error was unexpected, aborting...

This despite the fact that that test case isn't a 1cpu case, so that thread array never actually gets touched. Note that the index ("-1") is invalid. Something about the extra thread is triggering some kind of generation failure in gen_kobject_list.py it seems.

@andyross andyross added the bug The issue is a bug, or the PR is fixing a bug label Mar 9, 2022
@andyross andyross self-assigned this Mar 9, 2022
@nashif nashif added priority: high High impact/importance bug area: Userspace Userspace labels Mar 9, 2022
@andyross
Copy link
Contributor Author

andyross commented Mar 9, 2022

@dcpleung caught that, when the number of threads is exactly 16 (the maximum representable in the mask when CONFIG_MAX_THREAD_BYTES=2) the thread addresses seen by gen_kobject_list.py in the zephyr_pre1.elf file don't match the ones in the final zephyr.elf. This causes the kobject lookup for the thread to fail, leading to that invalid index.

The proximate cause is that, indeed, include/linker/kobject-data.ld will emit the _thread_idx_map[] array only in the final link, which is clearly wrong.

Unfortunately quick initial attempts to unify the two link modes don't address the mismatch, and I have no explanation for why the total thread count would matter (seems like it should be failing always, but it isn't). Still looking, but we're close.

dcpleung added a commit to dcpleung/zephyr that referenced this issue Mar 15, 2022
In some circumstances, _thread_idx_map[] is all zero and the linker
decides to put it into BSS instead of DATA section. This results in
kernel objects being pushed away so the hash table is no longer
valid. This forces _thread_idx_map to be in the data section inside
the intermediate object file so it will be placed in the data
section in final binary.

Fixes zephyrproject-rtos#43618

Signed-off-by: Daniel Leung <[email protected]>
nashif pushed a commit that referenced this issue Mar 15, 2022
In some circumstances, _thread_idx_map[] is all zero and the linker
decides to put it into BSS instead of DATA section. This results in
kernel objects being pushed away so the hash table is no longer
valid. This forces _thread_idx_map to be in the data section inside
the intermediate object file so it will be placed in the data
section in final binary.

Fixes #43618

Signed-off-by: Daniel Leung <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Userspace Userspace bug The issue is a bug, or the PR is fixing a bug priority: high High impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants