C standard library <time.h> functions and structures not available when using POSIX API

[vla3913]

Describe the bug
C standard library time.h functions and structures are not available when using CONFIG_POSIX_API=y

To Reproduce
Steps to reproduce the behavior:

1. cd samples/posix/gettimeofday
2. modify src/main:

#include <time.h>

// ...

void main(void)
{
    time_t now = time(NULL);
    struct tm tm_now;
    localtime_r(&now, &tm_now);
    printf("Time: %s\n", asctime(&tm_now));
    // ...
}

3. west build -b mimxrt1064_evk
4. See error and warnings (struct tm is of unknown size and implicit declarations)

Expected behavior
Functions and structures are correctly defined and no error or warning shows up.

Impact
I am trying to port a library to Zephyr and I need to use both posix time functions and C time functions.

Screenshots or console output

../src/main.c: In function 'main':
../src/main.c:18:15: warning: implicit declaration of function 'time' [-Wimplicit-function-declaration]
   18 |  time_t now = time(NULL);
      |               ^~~~
../src/main.c:19:12: error: storage size of 'tm_now' isn't known
   19 |  struct tm tm_now;
      |            ^~~~~~
../src/main.c:20:2: warning: implicit declaration of function 'localtime_r' [-Wimplicit-function-declaration]
   20 |  localtime_r(&now, &tm_now);
      |  ^~~~~~~~~~~
../src/main.c:22:23: warning: implicit declaration of function 'asctime' [-Wimplicit-function-declaration]
   22 |  printf("time: %s\n", asctime(&tm_now));
      |                       ^~~~~~~
../src/main.c:19:12: warning: unused variable 'tm_now' [-Wunused-variable]
   19 |  struct tm tm_now;
      |            ^~~~~~
ninja: build stopped: subcommand failed.

Environment (please complete the following information):
OS: Linux Ubuntu 18.04
Toolchain: Zephyr SDK version 0.11.2
tag: zephyr-v2.2.0

Additional context
N/A

[pabigot]

I'm willing to work this but I'd start by using the solution proposed in #18892 to avoid replicating and missing definitions declarations that come from the supporting toolchains. There were objections to that approach, so before I start I'd like input from @pfalcon to see if there's a more acceptable solution.

[pfalcon]

carlescufi assigned pabigot 22 minutes ago

@carlescufi, please at lease cc: me on any issues related to POSIX subsys, as I'm the codeowner for this subsystem.

[pfalcon]

west build -b mimxrt1064_evk

Please try to reproduce any issue with a standard test platform, which is qemu_x86. And trying that, you would hit #24746 , and that uncovered 3-4 more issues recursively.

So, investigation of this issue is so far blocked, pending subsiding of the issue wave rooted at #24746.

[pfalcon]

Ok, I can reproduce this with a patch like:

--- a/samples/posix/gettimeofday/src/main.c
+++ b/samples/posix/gettimeofday/src/main.c
@@ -8,11 +8,15 @@
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/time.h>
+#include <time.h>
 #include <unistd.h>
 
 int main(void)
 {
        struct timeval tv;
+       time_t now = time(NULL);
+       struct tm tm_now;
+       localtime_r(&now, &tm_now);
 

[pfalcon]

So, let's investigate and formulate what is the root of the problem. It turns out to be:

<time.h> header is defined by the ANSI/ISO C standard, and extended by the POSIX standard. This can be see at e.g. https://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_getres.html , where there's a [CX] mark with a hyperlink leading to the description that it's "Extension to the ISO C standard".

And the issue we're facing now is that Zephyr's POSIX subsys defined its own version of <time.h> header (at include/posix/time.h), which includes POSIX extensions, but lacks ISO-C standard things.

Let's then remember the invariant we follow: Newlib libc as the model we follow for integration between libc and POSIX subsys. So, let's look what Newlib has in its <time.h>.

Turns out, it has POSIX extensions in it already. As an example, I'm looking at zephyr-sdk-0.11.2/x86_64-zephyr-elf/x86_64-zephyr-elf/include/time.h, which has clock_settime/clock_gettime/timer_create/nanosleep/etc.

Thus, we make following conjecture on how to resolve the issue:

Approach 1: Zephyr's include/posix/time.h should be removed completely. Instead, libc's <time.h> header should be used right away, as that already contains POSIX extensions (minlibc's time.h thus will need to be extended, more specifically, most of include/posix/time.h's content migrated to it).

For risk managements, let's also stipulate:

Approach 2: Only if, while executing Approach 1, it will be found that Newlib's <time.h> misses some of POSIX functionality which we already implement, only then we'll proceed to establish a process of extending libc's headers in POSIX subsystem (for which we apparently would need to use #include_next , given the naming "conflict", and it would be the first proven case when #include_next is needed).

To mention other known invariants: a) the solution should work with both Newlib and minimal libc (which usually means that minimal libc header should grow their functionality, in a way consistent with how Newlib implements it); b) to establish that, a dedicated sample/test should be added (for example, samples/posix/gettimeofday isn't indeeded to be used without Newlib).

[pfalcon]

@pabigot: So, please let me know if you agree with the above analysis and implementation approach and would be interested in executing it.

(To clarify, if you don't agree, then I would still insist that it's correct approach, consistent with the established process of elaborating the Zephyr POSIX subsystem, and would queue up its execution on my side.)

Thanks.

[pabigot]

@pfalcon As I read your proposal it's consistent with my position in #18892 that we shouldn't be duplicating material we can get from existing headers.

I'll give a shot at implementing a solution following this approach.

[pfalcon]

@pfalcon As I read your proposal it's consistent with my position in #18892 that we shouldn't be duplicating material we can get from existing headers.

Yes, except that I consider that we should whenever possible use either libc's header or POSIX subsys header, and avoid #include_next until we're cornered into situation that we have (big enough) libc header, and need to extend it on POSIX side (which case I didn't see so far).

I'll give a shot at implementing a solution following this approach.

Thanks!

[pabigot]

@pfalcon As I read your proposal it's consistent with my position in #18892 that we shouldn't be duplicating material we can get from existing headers.

Yes, except that I consider that we should whenever possible use either libc's header or POSIX subsys header

I intended that to be covered by avoiding duplicating material we can get from using existing headers.

I immediately hit a roadblock, in that include/posix/time.h with CONFIG_POSIX=y can't just include <time.h> because it finds itself, since ${ZEPHYR_BASE}/include/posix is added as an include path. (That file also can't just be removed, because it's referred to as <posix/time.h> in a variety of places.)

It can't just #include_next <time.h> because it will still find itself, because that's not the path it used to find itself the first time.

But what it can do is in https://github.com/pabigot/zephyr/commits/nordic/pr/24746 which also adds the problematic API calls to the test. This passes the test (and is on top of #24746 to make the test pass in the first place).

The problematic API will not work with minimal libc because the corresponding functions are not implemented.

I have not undertaken to review all the remaining material in <posix/time.h> to determine what's redundant and what isn't. That's beyond the scope of what I can take on, given my nearly complete lack of familiarity with the Zephyr POSIX infrastructure.

So I'm going to have to hand it back to @pfalcon: a "fix" is trivial, the cleanup related to the fix is not. How much cleanup to do is a maintainer decision.

[pfalcon]

I intended that to be covered by avoiding duplicating material we can get from using existing headers.

If only it was that simple (as if we could "cherry-pick" what we get from existing header, and what not).

I immediately hit a roadblock,

Good start, welcome to POSIX subsys maintenance.

in that include/posix/time.h with CONFIG_POSIX=y can't just include <time.h> because it finds itself, since ${ZEPHYR_BASE}/include/posix is added as an include path.

That's why they invented #include_next, right?

(That file also can't just be removed, because it's referred to as <posix/time.h> in a variety of places.)

Oh, so those places would need to be fixed! (All as an experiment, whether that would fly or not).

It can't just #include_next <time.h> because it will still find itself, because that's not the path it used to find itself the first time.

Oh, looks like #include_next isn't as magical as it might seem.

But what it can do

I would need to sit and ponder what it does, but intuitive picture from a quick look is that it's roundabout way to redirect to Newlib's time.h.

The problematic API will not work with minimal libc because the corresponding functions are not implemented.

And yet we'll need to do something about minlibc, to make it clear that implementations of those functions should be dealt with on minlibc side (and not somewhere else). An obvious way to do that would be to just add declaration/prototypes to minlibc's time.h, but then we'll get comments from benevolent reviewers that we make "false advertising" for Zephyr...

That's beyond the scope of what I can take on, given my nearly complete lack of familiarity with the Zephyr POSIX infrastructure.

And I would need to deal with those, including stuff which is not in Zephyr's tree...

So I'm going to have to hand it back to @pfalcon: a "fix" is trivial, the cleanup related to the fix is not. How much cleanup to do is a maintainer decision.

"Alleged fix". And sounds good. And I feel that you're finally experienced from I tried to communicate previously: Zephyr POSIX subsys is in a state of subtle equilibrium, if not to say that it's a house of card, where any seemingly minor change may lead to cascade of failures, and even if it didn't immediately, it's no guarantee it won't, with some outside project. And saying "ain't broken, ain't fix" has very concrete meaning, why I wasn't exactly happy about brave proposals like #18892. Now that it's shown to be broken, we surely need to fix something, but I apologize in advance that I would start with trying to understand why we would unavoidably need posix/time.h in the first place, instead of just applying confusingly looking workaround and call it done.

Thanks for giving this triage and tentative fix anyway!

[carlescufi]

carlescufi assigned pabigot 22 minutes ago

@carlescufi, please at lease cc: me on any issues related to POSIX subsys, as I'm the codeowner for this subsystem.

Will do from now on, sorry.

[pfalcon]

Some update here: I didn't have a chance to further look into this, given that this is a "new" issue, and POSIX subsystem has a baggage of known issues and technical debt, and their priority-adjusted-for-wait-time is higher than this.

Given that the POSIX subsys is itself experimental and doesn't offer complete coverage of POSIX APIs/headers anyway, I'd be ok to lower priority for this ticket for the 2.3 release time. cc @carlescufi

[carlescufi]

Given that the POSIX subsys is itself experimental and doesn't offer complete coverage of POSIX APIs/headers anyway, I'd be ok to lower priority for this ticket for the 2.3 release time. cc @carlescufi

Thank you @pfalcon. I did that and this helps the release indeed.

[github-actions]

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.