SDK 2.1.0: clang 19.1.1 picolibc_interface compile error

[geurtv]

pico-sdk 2.1.0 compiles fine with clang 18.1.3 (LVM-ET-Arm-18.1.3) but with clang 19.1.1 (LLVM-ET-Arm-19.1.1) I get the following compile error:

pico-sdk/2.1.0/src/rp2_common/pico_clib_interface/picolibc_interface.c:102:83: error: use of undeclared identifier 'CLOCKS_PER_SEC'
  102 |     tms->tms_utime = (clock_t)(to_us_since_boot(get_absolute_time()) / (1000000 / CLOCKS_PER_SEC));
      |                                                                                   ^

I don't know what a correct fix is. For example, if I add the following code to picolibc_interface.c:

#include <time.h>

It compiles and CLOCKS_PER_SEC == 1000000.

clang 18's time.h includes machine/time.h, clang 19's does not. So if instead I add:

#include <machine/time.h>
#include <time.h>

It again compiles, but now CLOCKS_PER_SEC == 100 (same as clang 18).

[lurch]

I've not used clang myself, but is https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_clib_interface/include/llvm_libc/sys/times.h#L12 relevant here?

[geurtv]

I assume llvm_libc/sys/times.h will only be included for clang+llvm_libc (and not clang+picolibc), but I guess it does indicate the value for CLOCKS_PER_SEC is expected to be 100. So that leaves the question what the correct include is to get CLOCKS_PER_SEC == 100 in picolibc_interface.c.

[theofabilous]

Related commit: d6e3fa0
I'm almost certain the correct include is <machine/time.h> followed by <time.h>. In both LLVM and GCC embedded toolchains, machine/time.h contains the following definition:

// #if __arm__ ...
#define _CLOCKS_PER_SEC_ 100

and, in the GCC toolchain and earlier versions of LLVM ET, time.h then includes that file and defines

#define CLOCKS_PER_SEC _CLOCKS_PER_SEC_

For reference, this PR picolibc/picolibc#771 in picolibc both removes the inclusion of machine/time.h from time.h, and the inclusion of time.h in sys/time.h (related issue: picolibc/picolibc#800).

So now including sys/time.h doesn't bring in the definition of CLOCKS_PER_SEC, and just including time.h brings in the wrong definition of CLOCKS_PER_SEC (if machine/time.h isn't included beforehand), because it defaults to the XSI-conformant value if not defined:

#ifndef _CLOCKS_PER_SEC_
#define _CLOCKS_PER_SEC_ 1000000
#endif

It's my understanding that sys/time.h shouldn't include time.h according to POSIX/C but I'm not sure why the machine/time.h include was removed... might be an upstream (picolibc) problem. But the pico sdk definitely shouldn't include sys/time.h expecting it to define CLOCKS_PER_SEC, time.h is supposed to do that according to POSIX

Note: This was supposedly accounted for in SDK 2.0.0 (see #1781, fixed in another commit that I can't find) but from what I understand that was only for pico_runtime whereas the problem here is in pico_clib_interface

[kilograham]

merged into develop