zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.01k stars 6.16k forks source link

multipe tests/posix & tests/lib/c_lib/thrd build failures due to missing time.h definitions #75205

Open hakehuang opened 3 days ago

hakehuang commented 3 days ago

Describe the bug

build failure in the following platforms:

To Reproduce Steps to reproduce the behavior:

twister -i -p mimxrt1170_evk/mimxrt1176/cm7 -s portability.posix.headers.newlib.with_posix_api

Expected behavior build PASS

Impact POSIX compatible ~190 failures in weekly CI Failures in PRs CI when these tests are triggered

Logs and console output

modules/crypto/mbedtls/library/platform_util.c
In file included from zephyr/include/zephyr/posix/time.h:116,
                 from zephyr/include/zephyr/posix/time.h:12,
                 from zephyr/include/zephyr/net/socket_select.h:22,
                 from zephyr/include/zephyr/posix/sys/select.h:10,
                 from zephyr-sdk-0.16.8/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/sys/types.h:50,
                 from zephyr-sdk-0.16.8/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/stdio.h:61,
                 from modules/crypto/mbedtls/include/mbedtls/platform.h:58,
                 from modules/crypto/mbedtls/library/platform_util.c:26:
zephyr-sdk-0.16.8/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/time.h:56:1: error: unknown type name 'clock_t'
   56 | clock_t    clock (void);
      | ^~~~~~~
zephyr-sdk-0.16.8/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/time.h:30:1: note: 'clock_t' is defined in header '<time.h>'; did you forget to '#include <time.h>'?
   29 | #include <sys/timespec.h>
  +++ |+#include <time.h>
   30 | 
In file included from zephyr/include/zephyr/posix/posix_types.h:15,
                 from zephyr/include/zephyr/posix/time.h:61:
zephyr-sdk-0.16.8/arm-zephyr-eabi/arm-zephyr-eabi/sys-include/sys/_pthreadtypes.h:182:3: error: unknown type name 'clock_t'
  182 |   clock_t  clock;             /* specifiy clock for timeouts */
      |   ^~~~~~~
zephyr/include/zephyr/posix/posix_types.h:76:9: error: unknown type name 'clockid_t'
   76 |         clockid_t clock;
      |         ^~~~~~~~~
zephyr/include/zephyr/posix/time.h:93:19: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
   93 | int clock_gettime(clockid_t clock_id, struct timespec *ts);
      |                   ^~~~~~~~~
      |                   clock_pfd_t
zephyr/include/zephyr/posix/time.h:94:18: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
   94 | int clock_getres(clockid_t clock_id, struct timespec *ts);
      |                  ^~~~~~~~~
      |                  clock_pfd_t
zephyr/include/zephyr/posix/time.h:95:19: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
   95 | int clock_settime(clockid_t clock_id, const struct timespec *ts);
      |                   ^~~~~~~~~
      |                   clock_pfd_t
zephyr/include/zephyr/posix/time.h:96:36: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
   96 | int clock_getcpuclockid(pid_t pid, clockid_t *clock_id);
      |                                    ^~~~~~~~~
      |                                    clock_pfd_t
zephyr/include/zephyr/posix/time.h:98:18: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
   98 | int timer_create(clockid_t clockId, struct sigevent *evp, timer_t *timerid);
      |                  ^~~~~~~~~
      |                  clock_pfd_t
zephyr/include/zephyr/posix/time.h:105:21: error: unknown type name 'clockid_t'; did you mean 'clock_pfd_t'?
  105 | int clock_nanosleep(clockid_t clock_id, int flags,
      |                     ^~~~~~~~~
      |                     clock_pfd_t
In file included from zephyr/include/zephyr/sys/fdtable.h:13,
                 from zephyr/include/zephyr/net/socket_select.h:26:
zephyr/include/zephyr/fs/fs.h:368:1: error: unknown type name 'ssize_t'; did you mean '_ssize_t'?
  368 | ssize_t fs_read(struct fs_file_t *zfp, void *ptr, size_t size);
      | ^~~~~~~
      | _ssize_t
zephyr/include/zephyr/fs/fs.h:389:1: error: unknown type name 'ssize_t'; did you mean '_ssize_t'?
  389 | ssize_t fs_write(struct fs_file_t *zfp, const void *ptr, size_t size);
      | ^~~~~~~
      | _ssize_t
zephyr/include/zephyr/fs/fs.h:409:36: error: unknown type name 'off_t'; did you mean '_off_t'?
  409 | int fs_seek(struct fs_file_t *zfp, off_t offset, int whence);
      |                                    ^~~~~
      |                                    _off_t
zephyr/include/zephyr/fs/fs.h:425:1: error: unknown type name 'off_t'; did you mean '_off_t'?
  425 | off_t fs_tell(struct fs_file_t *zfp);
      | ^~~~~
      | _off_t
zephyr/include/zephyr/fs/fs.h:447:40: error: unknown type name 'off_t'; did you mean '_off_t'?
  447 | int fs_truncate(struct fs_file_t *zfp, off_t length);
      |                                        ^~~~~
      |                                        _off_t
zephyr/include/zephyr/sys/fdtable.h:48:17: error: expected specifier-qualifier-list before 'ssize_t'
   48 |                 ssize_t (*read)(void *obj, void *buf, size_t sz);
      |                 ^~~~~~~
zephyr/include/zephyr/sys/fdtable.h:52:17: error: expected specifier-qualifier-list before 'ssize_t'
   52 |                 ssize_t (*write)(void *obj, const void *buf, size_t sz);
      |                 ^~~~~~~
ninja: build stopped: subcommand failed.```

Environment (please complete the following information):

Additional info Introduced by #73978 PR is not bisectable

Edited by @aescolar to encompass all cases of the same issue

cfriedt commented 3 days ago

It's not really clear to me how commit a9a909c558c8c2069742aeee55aba831740826cd could affect anything to do with mbedtls or posix headers (so I think there was a misstep in the git bisect, likely missing a west update), but this test does fail to build using the command below on main, so I'll continue to debug.

twister -i -p mimxrt1170_evk/mimxrt1176/cm7 -s portability.posix.headers.newlib.with_posix_api
cfriedt commented 3 days ago

A bisect at my end identified 94d712e5cfe001996d7280c028e0e0303cf7b747 as the offending commit, which is also just as arbitrary and unrelated.

The search continues!

ithinuel commented 1 day ago

The bisect I ran points the same PR (https://github.com/zephyrproject-rtos/zephyr/pull/73978) however, the first commit of that PR does not break posix thing but fdtable.c.

The error is:

.../zephyr/lib/os/fdtable.c: In function 'zvfs_rw':
.../zephyr/lib/os/fdtable.c:315:41: error: 'const struct fd_op_vtable' has no member named 'write_offset'; did you mean 'write_offs'?
  315 |                 if (fdtable[fd].vtable->write_offset == NULL) {
      |                                         ^~~~~~~~~~~~
      |                                         write_offs
.../zephyr/lib/os/fdtable.c:319:51: error: 'const struct fd_op_vtable' has no member named 'write_offset'; did you mean 'write_offs'?
  319 |                         res = fdtable[fd].vtable->write_offset(fdtable[fd].obj, buf, sz,
      |                                                   ^~~~~~~~~~~~
      |                                                   write_offs
.../zephyr/lib/os/fdtable.c:327:51: error: 'const struct fd_op_vtable' has no member named 'read_offset'; did you mean 'read_offs'?
  327 |                         res = fdtable[fd].vtable->read_offset(fdtable[fd].obj, buf, sz,
      |                                                   ^~~~~~~~~~~
      |                                                   read_offs
.../zephyr/lib/os/fdtable.c:335:1: error: label 'unlock' defined but not used [-Werror=unused-label]
  335 | unlock:
      | ^~~~~~

the following commits in that PR do affect posix related implementation in various ways so my guess would be that it's more error compounding in what's been reported by @hakehuang

EDIT: this error gets fixed in a later commit in that same PR: https://github.com/cfriedt/zephyr/commit/88e631dc82a129853542c3dfbeadb995ac13714f#diff-c5e475ef95ce582f4a29b7c9669794a903686ea1d141f6095239e51176c99a5e

EDIT2: after fixing this error in the commit that introduced it, a new bisect points to https://github.com/zephyrproject-rtos/zephyr/pull/73978/commits/df00883bfb5682e5113675adb6390e1ff5d02a7a which also isn't directly related to the error reported here. Not having all commits to build makes git bisect a bit less useful to identify the offending commit.

duda-patryk commented 13 hours ago

I have the same problem. The sys/types.h from Newlib includes sys/select.h (provided by Zephyr), when __BSD_VISIBLE is set to 1. The sys/select.h later includes zephyr/net/socket_select.h which includes time.h and zephyr/sys/fdtable.h, which need off_t, ssize_t and clockid_t.

Missing types should be provided by sys/types.h, but sys/select.h is included before these types are defined.

Including posix_features.h using -imacro doesn't solve the issue. We can't also rely on POSIX headers from Newlib, because some types have different representation (e.g. pthread_rwlockattr_t).

For me, creating sys/types.h with the following content worked (sys/types.h from Newlib never includes sys/select.h).

diff --git a/include/zephyr/posix/sys/types.h b/include/zephyr/posix/sys/types.h
new file mode 100644
index 00000000000..9a13cfbda64
--- /dev/null
+++ b/include/zephyr/posix/sys/types.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2024 Google LLC
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#ifndef ZEPHYR_INCLUDE_POSIX_SYS_TYPES_H_
+#define ZEPHYR_INCLUDE_POSIX_SYS_TYPES_H_
+
+#ifdef CONFIG_NEWLIB_LIBC
+
+#include <sys/features.h>
+#undef __BSD_VISIBLE
+
+#endif /* CONFIG_NEWLIB_LIBC */
+
+#include_next <sys/types.h>
+
+#endif /* ZEPHYR_INCLUDE_POSIX_SYS_TYPES_H_ */
cfriedt commented 5 hours ago

Just got back (I have quite a commute :-/). Looking at it again.

cfriedt commented 4 hours ago

I think part of the issue is that posix types are being used where they probably shouldn't be.

off_t, ssize_t and <sys/types.h> are all part of posix but are being used in fdtable.h (below the posix api). The problem is compounded because all of the fdtable.h consumers are also therefore forced to use posix types creating a bit of a layering rat's nest.

I have a fairly simple workaround (just define the types locally). They should be removed (after v3.7.0)

cfriedt commented 2 hours ago

Very strange how this somehow managed to go through CI without triggering any failures on the original set of PRs...