[PATCH 1/1] Use types.h and not time_types.h in sockios.h

[PATCH 1/1] Use types.h and not time_types.h in sockios.h

[Geoffrey Le Gourriérec]

This fixes builds for sh arch when libc is not using relevant
time data structures definitions for 32-bit machines. A previous
commit [1] provided a fix, that we seemed to slip through here.

As of the time of this writing, the bug was found with v5.10.7,
with uclibc 1.0.37 only (currently the only libc supporting sh
architecture). uclibc, unlike glibc or muslibc, does not seem to
have made the complete move to post-year-2038 world for 32-bit.
The issue can be reproduced as follows:

$ git clone git://git.busybox.net/buildroot
$ cd buildroot/
$ git checkout 742f37d
$ rm -rf board/qemu/sh4eb-r2d/patches/linux-headers/
$ sed -e '/BR2_GLOBAL_PATCH_DIR/d' -i configs/qemu_sh4eb_r2d_defconfig
$ make qemu_sh4eb_r2d_defconfig
$ make
$ ./output/images/start-qemu.sh

I tried to understand why the initial commit [1] did not prevent
this issue (see notes below), but failed to do so. If anybody can
shed some light on this, I'll gladly take it.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fc94cf2092c7c1267fa2deb8388d624f50eba808

Signed-off-by: Geoffrey Le Gourriérec <geoffrey.legourrierec@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>

---
For the record, here's the build-time error:

/usr/bin/make -j2 -C /builds/clumsyape/buildroot/output/build/uclibc-1.0.37 ARCH="sh" CROSS_COMPILE="/builds/clumsyape/buildroot/output/host/bin/sh4-buildroot-linux-uclibc-" UCLIBC_EXTRA_CFLAGS="" HOSTCC="/usr/bin/gcc"
make[1]: Entering directory '/builds/clumsyape/buildroot/output/build/uclibc-1.0.37'
  GEN libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelcond.h
  GEN libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelcond.h
  GEN libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelrobustlock.h
  GEN libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelrobustlock.h
In file included from /builds/clumsyape/buildroot/output/build/linux-headers-5.10.7/usr/include/asm/sockios.h:5,
                 from /builds/clumsyape/buildroot/output/build/linux-headers-5.10.7/usr/include/asm-generic/socket.h:6,
                 from /builds/clumsyape/buildroot/output/build/linux-headers-5.10.7/usr/include/asm/socket.h:1,
                 from ./include/bits/socket.h:360,
                 from ./include/sys/socket.h:39,
                 from ./include/netinet/in.h:24,
                 from ./include/resolv.h:57,
                 from ./libpthread/nptl/descr.h:36,
                 from ./libpthread/nptl/pthreadP.h:25,
                 from <stdin>:2:
/builds/clumsyape/buildroot/output/build/linux-headers-5.10.7/usr/include/linux/time_types.h:8:2: error: unknown type name '__kernel_time64_t'
    8 |  __kernel_time64_t       tv_sec;                 /* seconds */
      |  ^~~~~~~~~~~~~~~~~
/builds/clumsyape/buildroot/output/build/linux-headers-5.10.7/usr/include/linux/time_types.h:32:2: error: unknown type name '__kernel_old_time_t'
   32 |  __kernel_old_time_t tv_sec;  /* seconds */
      |  ^~~~~~~~~~~~~~~~~~~
libpthread/nptl/sysdeps/unix/sysv/linux/Makefile.commonarch:135: recipe for target 'libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelrobustlock.h' failed
make[1]: *** [libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelrobustlock.h] Error 1
make[1]: Leaving directory '/builds/clumsyape/buildroot/output/build/uclibc-1.0.37'

I did a quick header dependency analysis starting from __kernel_time64_t
(one of the offending types), but could "follow up" to linux/time_types.h
as expected; so I fail to understand how this could break. What's even
more confusing is linux/time_types.h includes linux/types.h itself.
---
 arch/sh/include/uapi/asm/sockios.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/include/uapi/asm/sockios.h b/arch/sh/include/uapi/asm/sockios.h
index ef01ced9e169..d97d14685305 100644
--- a/arch/sh/include/uapi/asm/sockios.h
+++ b/arch/sh/include/uapi/asm/sockios.h
@@ -2,7 +2,7 @@
 #ifndef __ASM_SH_SOCKIOS_H
 #define __ASM_SH_SOCKIOS_H
 
-#include <linux/time_types.h>
+#include <linux/types.h>
 
 /* Socket-level I/O control calls. */
 #define FIOGETOWN	_IOR('f', 123, int)
-- 
2.17.1

Re: [PATCH 1/1] Use types.h and not time_types.h in sockios.h

[Arnd Bergmann]

On Sat, Jan 23, 2021 at 5:56 PM Geoffrey Le Gourriérec
<geoffrey.legourrierec@gmail.com> wrote:
>
> I did a quick header dependency analysis starting from __kernel_time64_t
> (one of the offending types), but could "follow up" to linux/time_types.h
> as expected; so I fail to understand how this could break. What's even
> more confusing is linux/time_types.h includes linux/types.h itself.

Could you try producing a preprocessed version of the source file using 'gcc -E'
instead of 'gcc -c', using the same arguments as in the failing command?

The output of that should help pinpoint what is going wrong.

I'm fairly sure the patch you sent gets you back to the original problem:
Since it avoids including linux/time_types.h, the compiler no longer
sees the line that fails to build, but at the same time you can not
actually use the SIOCGSTAMP_OLD and SIOCGSTAMPNS_OLD
constants any more, as the structures will be undefined.

      Arnd

Re: [PATCH 1/1] Use types.h and not time_types.h in sockios.h

[Geoffrey Le Gourriérec]

> On Sat, Jan 23, 2021 at 5:56 PM Geoffrey Le Gourriérec
> <geoffrey.legourrierec@gmail.com> wrote:
> >
> > I did a quick header dependency analysis starting from __kernel_time64_t
> > (one of the offending types), but could "follow up" to linux/time_types.h
> > as expected; so I fail to understand how this could break. What's even
> > more confusing is linux/time_types.h includes linux/types.h itself.
>
> Could you try producing a preprocessed version of the source file using 'gcc -E'
> instead of 'gcc -c', using the same arguments as in the failing command?
>
> The output of that should help pinpoint what is going wrong.

Hi Arnd,

Here is the preprocessed
uclibc-1.0.37/libpthread/nptl/sysdeps/unix/sysv/linux/lowlevelrobustlock.h
:
https://pastebin.com/ypWCZK83

Considering the following dependency chain:
include/uapi/asm-generic/posix_types.h (__kernel_time64_t and
__kernel_old_time_t defined here)
  -- included by: include/uapi/asm/posix_types_32.h (within #ifndef
__ASM_SH_POSIX_TYPES_32_H)

...It looks like we do not enter the __ASM_SH_POSIX_TYPES_32_H include guard.
Searching for that in the uclibc source code, I fall upon this header:
https://repo.or.cz/uclibc-ng.git/blob/ab1dd83bec59c9e65c31efd6e887182948f627be:/libc/sysdeps/linux/sh/bits/kernel_types.h
...with a preliminary comment that seems quite explicit (I am not
familiar at all with the
subject, sorry if I'm perhaps stating the obvious):

/* Note that we use the exact same include guard #define names
* as asm/posix_types.h. This will avoid gratuitous conflicts
* with the posix_types.h kernel header, and will ensure that
* our private content, and not the kernel header, will win.
* -Erik
*/
#if !defined __ASM_SH_POSIX_TYPES_H && !defined __ASM_SH_POSIX_TYPES_32_H
#define __ASM_SH_POSIX_TYPES_H
#define __ASM_SH_POSIX_TYPES_32_H

> I'm fairly sure the patch you sent gets you back to the original problem:
> Since it avoids including linux/time_types.h, the compiler no longer
> sees the line that fails to build, but at the same time you can not
> actually use the SIOCGSTAMP_OLD and SIOCGSTAMPNS_OLD
> constants any more, as the structures will be undefined.

That is correct, my patch just fixes the symptom. I plan to come back
to the Buildroot
folks with a proper patch once we've figured something out.

Thanks for your review,

-- 
Geoffrey


Le dim. 24 janv. 2021 à 11:56, Arnd Bergmann <arnd@kernel.org> a écrit :
>
> On Sat, Jan 23, 2021 at 5:56 PM Geoffrey Le Gourriérec
> <geoffrey.legourrierec@gmail.com> wrote:
> >
> > I did a quick header dependency analysis starting from __kernel_time64_t
> > (one of the offending types), but could "follow up" to linux/time_types.h
> > as expected; so I fail to understand how this could break. What's even
> > more confusing is linux/time_types.h includes linux/types.h itself.
>
> Could you try producing a preprocessed version of the source file using 'gcc -E'
> instead of 'gcc -c', using the same arguments as in the failing command?
>
> The output of that should help pinpoint what is going wrong.
>
> I'm fairly sure the patch you sent gets you back to the original problem:
> Since it avoids including linux/time_types.h, the compiler no longer
> sees the line that fails to build, but at the same time you can not
> actually use the SIOCGSTAMP_OLD and SIOCGSTAMPNS_OLD
> constants any more, as the structures will be undefined.
>
>       Arnd

Re: [PATCH 1/1] Use types.h and not time_types.h in sockios.h

[Arnd Bergmann]

On Sun, Jan 24, 2021 at 5:38 PM Geoffrey Le Gourriérec
<geoffrey.legourrierec@gmail.com> wrote:
> > On Sat, Jan 23, 2021 at 5:56 PM Geoffrey Le Gourriérec <geoffrey.legourrierec@gmail.com> wrote:

> ...It looks like we do not enter the __ASM_SH_POSIX_TYPES_32_H include guard.
> Searching for that in the uclibc source code, I fall upon this header:
> https://repo.or.cz/uclibc-ng.git/blob/ab1dd83bec59c9e65c31efd6e887182948f627be:/libc/sysdeps/linux/sh/bits/kernel_types.h
> ...with a preliminary comment that seems quite explicit (I am not
> familiar at all with the
> subject, sorry if I'm perhaps stating the obvious):
>
> /* Note that we use the exact same include guard #define names
> * as asm/posix_types.h. This will avoid gratuitous conflicts
> * with the posix_types.h kernel header, and will ensure that
> * our private content, and not the kernel header, will win.
> * -Erik
> */
> #if !defined __ASM_SH_POSIX_TYPES_H && !defined __ASM_SH_POSIX_TYPES_32_H
> #define __ASM_SH_POSIX_TYPES_H
> #define __ASM_SH_POSIX_TYPES_32_H

Ok, so I guess this means that it's just a bug in uclibc. Most likely this code
dates back a very long time, back when the asm/posix_types.h in the
kernel was in fact defining the same types as user space. Since it now
only contains the __kernel_* prefixed types, you can probably just
remove this hack. If that doesn't work, then uclibc needs to be changed to
define the same types as the kernel header.

        Arnd