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.

Sign up for GitHub

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

Jump to bottom

GCC 13 compiler warning in kernel/userspace.c #72430

Closed
davidschneider-cpi opened this issue May 7, 2024 · 3 comments · Fixed by #72781
Closed

GCC 13 compiler warning in kernel/userspace.c #72430

davidschneider-cpi opened this issue May 7, 2024 · 3 comments · Fixed by #72781
Assignees
@dcpleung
Labels
area: Kernel area: Userspace Userspace bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@davidschneider-cpi
Copy link
Contributor

Describe the bug
GCC 13 has improved analysis which raises a warning due to reading address outside of _thread_idx_map.

If CONFIG_MAX_THREAD_BYTES is not a multiple of 4 the cast to uint32_t* will read beyond array boundaries.

To Reproduce

Steps to reproduce the behavior:

ZEPHYR_TOOLCHAIN_VARIANT=gnuarmemb GNUARMEMB_TOOLCHAIN_PATH=/opt/arm/arm-gnu-toolchain-13.2.Rel1-x86_64-arm-none-eabi ./scripts/twister -T tests/kernel/common -v --hardware-map map.yml  --device-testing -c -s tests/kernel/common/kernel.common

Expected behavior

Build without warnings.

Logs and console output

In file included from /home/david/zephyrproject/zephyr/include/zephyr/arch/arm/arch.h:34,
                 from /home/david/zephyrproject/zephyr/include/zephyr/arch/cpu.h:19,
                 from /home/david/zephyrproject/zephyr/include/zephyr/kernel_includes.h:36,
                 from /home/david/zephyrproject/zephyr/include/zephyr/kernel.h:17,
                 from /home/david/zephyrproject/zephyr/kernel/userspace.c:8:
In function 'sys_clear_bit',
    inlined from 'sys_bitfield_clear_bit' at /home/david/zephyrproject/zephyr/include/zephyr/arch/common/sys_bitops.h:71:2,
    inlined from 'thread_idx_alloc' at /home/david/zephyrproject/zephyr/kernel/userspace.c:280:4,
    inlined from 'z_object_alloc' at /home/david/zephyrproject/zephyr/kernel/userspace.c:401:8:
/home/david/zephyrproject/zephyr/include/zephyr/arch/common/sys_bitops.h:33:18: error: array subscript 'uint32_t {aka volatile unsigned int}[0]' is partly outside array bounds of 'uint8_t[3]' {aka 'unsigned char[3]'} [-Werror=array-bounds=]
   33 |         uint32_t temp = *(volatile uint32_t *)addr;
      |                  ^~~~
/home/david/zephyrproject/zephyr/kernel/userspace.c: In function 'z_object_alloc':
/home/david/zephyrproject/zephyr/kernel/userspace.c:77:16: note: object '_thread_idx_map' of size 3
   77 | extern uint8_t _thread_idx_map[CONFIG_MAX_THREAD_BYTES];
      |                ^~~~~~~~~~~~~~~
In function 'sys_clear_bit',
    inlined from 'sys_bitfield_clear_bit' at /home/david/zephyrproject/zephyr/include/zephyr/arch/common/sys_bitops.h:71:2,
    inlined from 'thread_idx_alloc' at /home/david/zephyrproject/zephyr/kernel/userspace.c:280:4,
    inlined from 'z_object_alloc' at /home/david/zephyrproject/zephyr/kernel/userspace.c:401:8:
/home/david/zephyrproject/zephyr/include/zephyr/arch/common/sys_bitops.h:35:36: error: array subscript 'uint32_t {aka volatile unsigned int}[0]' is partly outside array bounds of 'uint8_t[3]' {aka 'unsigned char[3]'} [-Werror=array-bounds=]
   35 |         *(volatile uint32_t *)addr = temp & ~(1 << bit);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/home/david/zephyrproject/zephyr/kernel/userspace.c: In function 'z_object_alloc':
/home/david/zephyrproject/zephyr/kernel/userspace.c:77:16: note: object '_thread_idx_map' of size 3
   77 | extern uint8_t _thread_idx_map[CONFIG_MAX_THREAD_BYTES];
      |                ^~~~~~~~~~~~~~~
``

**Environment (please complete the following information):**
 - OS: Ubuntu 22.04
 - Toolchain: gnuarmemb (13.2.Rel1)
 - d77dc62d196b8f5f58622a05414002cbf40285ba
@davidschneider-cpi davidschneider-cpi added the bug The issue is a bug, or the PR is fixing a bug label May 7, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 7, 2024

Hi @davidschneider-cpi! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

@aescolar aescolar changed the title GCC 13 compiler warning GCC 13 compiler warning in kernel/userspace.c May 7, 2024
@nashif nashif added the priority: medium Medium impact/importance bug label May 7, 2024
@nashif
Copy link
Member

nashif commented May 7, 2024

@ceolin FYI

@dcpleung
Copy link
Member

Hm... I will need a GCC 13/14 enabled Zephyr SDK to properly test any changes.

dcpleung added a commit to dcpleung/zephyr that referenced this issue May 15, 2024
@dcpleung
kernel: userspace: manipulate _thread_idx_map on per-byte basis
25a4aef 
The sys_bitfield_(clear/set)_bit() work on pointer size element.
However, _thread_idx_map[] is a byte array. On little endian
systems, the bitops should work fine. However, on big endian
systems, changing the lower bits may actually be manipulating
memory outside the array when CONFIG_MAX_THREAD_BYTES is not
multiple of 4. So modify the code to perform bit ops on
a per-byte basis.

Fixes zephyrproject-rtos#72430

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
@dcpleung dcpleung mentioned this issue May 15, 2024
Merged
dcpleung added a commit to dcpleung/zephyr that referenced this issue May 15, 2024
@dcpleung
kernel: userspace: manipulate _thread_idx_map on per-byte basis
62fcb13 
The sys_bitfield_(clear/set)_bit() work on pointer size element.
However, _thread_idx_map[] is a byte array. On little endian
systems, the bitops should work fine. However, on big endian
systems, changing the lower bits may actually be manipulating
memory outside the array when CONFIG_MAX_THREAD_BYTES is not
multiple of 4. So modify the code to perform bit ops on
a per-byte basis.

Fixes zephyrproject-rtos#72430

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
jhedberg pushed a commit that referenced this issue May 18, 2024
@dcpleung @jhedberg
kernel: userspace: manipulate _thread_idx_map on per-byte basis
2ad265c 
The sys_bitfield_(clear/set)_bit() work on pointer size element.
However, _thread_idx_map[] is a byte array. On little endian
systems, the bitops should work fine. However, on big endian
systems, changing the lower bits may actually be manipulating
memory outside the array when CONFIG_MAX_THREAD_BYTES is not
multiple of 4. So modify the code to perform bit ops on
a per-byte basis.

Fixes #72430

Signed-off-by: Daniel Leung <daniel.leung@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dcpleung dcpleung

Labels
area: Kernel area: Userspace Userspace bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kernel: userspace: manipulate _thread_idx_map on per-byte basis dcpleung/zephyr
4 participants
@nashif @aescolar @dcpleung @davidschneider-cpi