Build newlib function read() and write() failed when enable userspace

[yingmingx]

Describe the bug

1. I'm try to test the stub in libc-hooks.c and test read() and write() in userspace. When read() is called, the verification function z_vrfy_z_zephyr_read_stdin() won't be called, it directly called the implementation function z_impl_zephyr_read_stdin(). and then page fault is caught. It is also the case when the write function is called.
2. When I directly call system call z_zephyr_read_stdin(), there is a build error.

To Reproduce Steps to reproduce the behavior:

1. enable CONFIG_NEWLIB_LIBC=y and CONFIG_TEST_USERSPACE=y 2.1 write a testcase and then call read() or write() or directly it will trigger page fault. 2.2 called system call z_zephyr_read_stdin() directly, it would be a link error.

Expected behavior

1. the read() and write() in newlib can invoke the verification function when userspace is enabled.
2. When call system call z_zephyr_read_stdin(), the compiler can link correctly.

Impact I met this problem when testing #38064

Logs and console output 2.1

START - test_newlib_stub_read
E: Page fault at address 0x133508 (error code 0x4)
E: Linear address not present in page tables
E: Access violation: user thread not allowed to read
E: PTE: not present
E: EAX: 0x00118910, EBX: 0x00000000, ECX: 0x00000000, EDX: 0x00000001
E: ESI: 0x00000000, EDI: 0x0012ffe8, EBP: 0x0012ffac, ESP: 0x0012ffa8
E: EFLAGS: 0x00000297 CS: 0x002b CR3: 0x001187a0
E: call trace:
E: EIP: 0x00104865
E:      0x001007c0 (0x118910)
E:      0x00104b35 (0x120033)
E:      0x0010119b (0x115064)
E: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0
E: Current thread: 0x117080 (test_newlib_stub_read)
E: Halting system

2.2

[ 78%] Linking C executable app_smem_unaligned_prebuilt.elf
/home/yingming/zephyr-sdk-0.13.0/x86_64-zephyr-elf/bin/../lib/gcc/x86_64-zephyr-elf/10.3.0/../../../../x86_64-zephyr-elf/bin/ld.bfd: ../app/libapp.a(main.c.obj): in function `z_zephyr_read_stdin':
/home/yingming/zephyrproject/zephyr/twister-out/qemu_x86/tests/lib/newlib/newlib_hook/libraries.libc.newlib.userspace/zephyr/include/generated/syscalls/libc-hooks.h:37: undefined reference to `z_impl_z_zephyr_read_stdin'
collect2: error: ld returned 1 exit status
zephyr/CMakeFiles/app_smem_unaligned_prebuilt.dir/build.make:111: recipe for target 'zephyr/app_smem_unaligned_prebuilt.elf' failed
make[2]: *** [zephyr/app_smem_unaligned_prebuilt.elf] Error 1
CMakeFiles/Makefile2:2532: recipe for target 'zephyr/CMakeFiles/app_smem_unaligned_prebuilt.dir/all' failed
make[1]: *** [zephyr/CMakeFiles/app_smem_unaligned_prebuilt.dir/all] Error 2
Makefile:90: recipe for target 'all' failed
make: *** [all] Error 2

Environment (please complete the following information):

• OS: Linux
• Toolchain: zephyr-sdk-0.13.0

[stephanosio]

I thought calling these from userspace worked (at least on ARM). Maybe this failure is x86-specific.

Needs more investigation.

[yingmingx]

When directly call read() in userspace on platform mps2_an385.

START - test_newlib_stub_read E: MPU FAULT E: Data Access Violation E: MMFAR Address: 0x20003c48 E: r0/a1: 0x200006b8 r1/a2: 0x00000001 r2/a3: 0x00000001 E: r3/a4: 0x000009e1 r12/ip: 0x0000aa9f r14/lr: 0x000009ef E: xpsr: 0x81000000 E: Faulting instruction address (r15/pc): 0x00002c42 E: >>> ZEPHYR FATAL ERROR 0: CPU exception on CPU 0 E: Current thread: 0x20000180 (test_newlib_stub_read) E: Halting system

[stephanosio]

Yup, this is a bug -- the newlib _write and _read hooks should indeed be calling the syscall functions and not the impls themselves.

They used to do that, but that got changed (by mistake) during the process of renaming functions around.

In https://github.com/zephyrproject-rtos/zephyr/commit/4344e27c267e1139c26dbd9cafcca4b3069ea821:

diff --git a/lib/libc/newlib/libc-hooks.c b/lib/libc/newlib/libc-hooks.c
index 578271916b..d6bef58d2b 100644
--- a/lib/libc/newlib/libc-hooks.c
+++ b/lib/libc/newlib/libc-hooks.c
@@ -153,7 +153,7 @@ int _read(int fd, char *buf, int nbytes)
 {
        ARG_UNUSED(fd);

-       return _zephyr_read_stdin(buf, nbytes);
+       return z_impl_zephyr_read_stdin(buf, nbytes);
 }
 FUNC_ALIAS(_read, read, int);

@@ -161,7 +161,7 @@ int _write(int fd, const void *buf, int nbytes)
 {
        ARG_UNUSED(fd);

-       return _zephyr_write_stdout(buf, nbytes);
+       return z_impl_zephyr_write_stdout(buf, nbytes);
 }
 FUNC_ALIAS(_write, write, int);

The importance of having the tests for all these cannot be overstated.

p.s. How did we manage to not notice this for over 2 years?