Using `signals` from `libc` is broken

[krish2718]

Describe the bug An application with CONFIG_POSIX_API which was working before the commit is now broken.

Zephyr's POSIX implementation doesn't support signal. But libc does support it, but after the change when CONFIG_POSIX_API is enabled, and if we want to use POSIX signals then its broken as in the list of include directories include/zephyr/posix/signal.h appears first before libc headers and that file doesn't has signal related definitions. (It only has definitions needed to support pthread_cond_signal).

To Reproduce Steps to reproduce the behavior: Clone https://github.com/krish2718/zephyr/tree/libc_signal and build samples/net/wifi (I have used VS code to build.

Expected behavior Successful build

Impact Existing application (downstream) is broken.

Logs and console output

FAILED: CMakeFiles/app.dir/src/wifi_test.c.obj 
ccache /home/tach/work/zephyr-sdk-0.15.2/arm-zephyr-eabi/bin/arm-zephyr-eabi-gcc -DKERNEL -DMBEDTLS_CONFIG_FILE=\"config-tls-generic.h\" -DNRF5340_XXAA_APPLICATION -DNRF_SKIP_FICR_NS_COPY_TO_RAM -D_FORTIFY_SOURCE=2 -D__LINUX_ERRNO_EXTENSIONS__ -D__PROGRAM_START -D__ZEPHYR__=1 -I/home/tach/work/upstream/zephyr/include/zephyr -I/home/tach/work/upstream/zephyr/include -I/home/tach/work/upstream/zephyr/samples/net/wifi/build/zephyr/include/generated -I/home/tach/work/upstream/zephyr/soc/arm/nordic_nrf/nrf53 -I/home/tach/work/upstream/zephyr/lib/libc/newlib/include -I/home/tach/work/upstream/zephyr/lib/util/fnmatch/. -I/home/tach/work/upstream/zephyr/soc/arm/nordic_nrf/common/. -I/home/tach/work/upstream/modules/hal/cmsis/CMSIS/Core/Include -I/home/tach/work/upstream/modules/hal/atmel/include -I/home/tach/work/upstream/modules/hal/nordic/nrfx -I/home/tach/work/upstream/modules/hal/nordic/nrfx/drivers/include -I/home/tach/work/upstream/modules/hal/nordic/nrfx/mdk -I/home/tach/work/upstream/zephyr/modules/hal_nordic/nrfx/. -I/home/tach/work/upstream/zephyr/include/zephyr/posix -I/home/tach/work/upstream/modules/crypto/mbedtls/include -I/home/tach/work/upstream/zephyr/modules/mbedtls/configs -Os -imacros /home/tach/work/upstream/zephyr/samples/net/wifi/build/zephyr/include/generated/autoconf.h -ffreestanding -fno-common -g -gdwarf-4 -fdiagnostics-color=always -mcpu=cortex-m33 -mthumb -mabi=aapcs -mfp16-format=ieee --sysroot=/home/tach/work/zephyr-sdk-0.15.2/arm-zephyr-eabi/arm-zephyr-eabi -imacros /home/tach/work/upstream/zephyr/include/zephyr/toolchain/zephyr_stdint.h -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wno-main -Wno-pointer-sign -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -Werror=implicit-int -fno-asynchronous-unwind-tables -fno-pie -fno-pic -fno-reorder-functions -fno-defer-pop -fmacro-prefix-map=/home/tach/work/upstream/zephyr/samples/net/wifi=CMAKE_SOURCE_DIR -fmacro-prefix-map=/home/tach/work/upstream/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/home/tach/work/upstream=WEST_TOPDIR -ffunction-sections -fdata-sections -specs=nano.specs -std=c99 -MD -MT CMakeFiles/app.dir/src/wifi_test.c.obj -MF CMakeFiles/app.dir/src/wifi_test.c.obj.d -o CMakeFiles/app.dir/src/wifi_test.c.obj -c /home/tach/work/upstream/zephyr/samples/net/wifi/src/wifi_test.c
/home/tach/work/upstream/zephyr/samples/net/wifi/src/wifi_test.c: In function 'main':
/home/tach/work/upstream/zephyr/samples/net/wifi/src/wifi_test.c:12:9: warning: implicit declaration of function 'signal' [-Wimplicit-function-declaration]
   12 |         signal(SIGINT, NULL);
      |         ^~~~~~
/home/tach/work/upstream/zephyr/samples/net/wifi/src/wifi_test.c:12:16: error: 'SIGINT' undeclared (first use in this function)
   12 |         signal(SIGINT, NULL);
      |                ^~~~~~
/home/tach/work/upstream/zephyr/samples/net/wifi/src/wifi_test.c:12:16: note: each undeclared identifier is reported only once for each function it appears in

Environment (please complete the following information):

• OS: LInux
• Toolchain; Zephyr SDK 0.15.2
• Commit SHA: https://github.com/krish2718/zephyr/tree/libc_signal

Additional context We could solve this in many ways:

1. Add signal support properly to Zephyr POSIX implementation (long term)
2. Tweak Cmake to include libc headers before Zephyr, if its possible, but I don't see prepend_include_directories in cmake.
3. As the commit log says, avoid using CONFIG_POSIX_API and instead individually enable POSIX_* modules as a hack.

I am leaning towards trying out #3 or #2 for short term.

@cfriedt @stephanosio @sachinthegreen

[cfriedt]

@krish2718 - please note, that POSIX signals have never been supported. They are not broken.

https://docs.zephyrproject.org/latest/services/portability/posix.html#units-of-functionality

[krish2718]

@krish2718 - please note, that POSIX signals have never been supported. They are not broken.

https://docs.zephyrproject.org/latest/services/portability/posix.html#units-of-functionality

Sure, not in Zephyr, but an application could still use libc signal API before that commit, so, in that sense a working application is now broken.

[cfriedt]

True signal() is also part of C99. However, I'm not entirely sure if C signals have ever been supported in Zephyr either.

Perhaps it's best to use an existing signalling mechanism that is supported in Zephyr and has test coverage in CI.

@stephanosio - I'll leave this one up to you.

https://en.cppreference.com/w/c/program/signal

[cfriedt]

Just an aside, but the use of signal() exists nowhere in Zephyr under tests/ or samples/ today. So there are no existing in-tree applications that are broken.

This issue is really only visible in a downstream application.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[krish2718]

Perhaps it's best to use an existing signalling mechanism that is supported in Zephyr and has test coverage in CI.

AFAIK we don't have anything supported now, so, should we extend this issue to add this? (else it gets closed automatically).

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[cfriedt]

Perhaps it's best to use an existing signalling mechanism that is supported in Zephyr and has test coverage in CI.

AFAIK we don't have anything supported now, so, should we extend this issue to add this? (else it gets closed automatically).

There are many existing signalling APIs supported currently, but the one that comes to mind immediately is k_poll_signal_raise().

See https://docs.zephyrproject.org/apidoc/latest/group__poll__apis.html

Per-thread signalling should be supported in POSIX soon with e.g. pthread_kill() but global (per process) signal() is likely out of scope.

See https://github.com/zephyrproject-rtos/zephyr/issues/51211

[krish2718]

I was looking at https://docs.zephyrproject.org/latest/services/portability/posix.html#posix-signals but looking at https://github.com/zephyrproject-rtos/zephyr/issues/51211 _POSIX_SIGNALS is what this bug refers to.

[cfriedt]

I think to fix the compiler warning is rather trivial (via #include_next <signal.h>), but there is the obvious caveat that even the ANSI C version of that function may have unsupported or unpredictable behaviour with side effects in Zephyr at the moment.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions[bot]]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[cfriedt]

I will add signal support to the kernel post 3.7, and that will cover ISO C, POSIX, and Realtime signals, so I can take this.