On Tue, Jan 3, 2023, at 17:47, Thorsten Glaser wrote:> Hi, > > I noticed a failure of mksh built with klibc on mips64el. > > The failing test, on a high level, is this: > > :>a > sleep 2 > :>b > test a -nt b > echo $? > > This is supposed to echo 1 (false) because a is not newer than b. > > The test code is roughly: > > // const char *opnd1 = "a"; > // const char *opnd2 = "b"; > // struct stat b1, b2; > // int s; > > return (test_stat(opnd1, &b1) == 0 && > (((s = test_stat(opnd2, &b2)) == 0 && > mtimecmp(&b1, &b2) > 0) || s < 0)); > > // #define st_mtimensec st_mtim.tv_nsec > // #define HAVE_ST_MTIMENSEC 1 > > static int > mtimecmp(const struct stat *sb1, const struct stat *sb2) > { > if (sb1->st_mtime < sb2->st_mtime) > return (-1); > if (sb1->st_mtime > sb2->st_mtime) > return (1); > #if HAVE_ST_MTIMENSEC > if (sb1->st_mtimensec < sb2->st_mtimensec) > return (-1); > if (sb1->st_mtimensec > sb2->st_mtimensec) > return (1); > #endif > return (0); > } > > The build+test was on mipsel-osuosl-03.debian.org with uname -a: >> Linux mipsel-osuosl-03 5.10.0-20-loongson-3 #1 SMP PREEMPT Debian 5.10.158-2 (2022-12-13) mips64 GNU/Linux > > On the same machine and during the same build, the test passed > with glibc, musl and dietlibc. > > Does this suffice to begin debugging?I don't yet see what the problem is, but it's worth noting that mips64 is the only architecture using 'unsigned int' for the st_mtime field, while everything else uses 'long'. There is a good chance that the different type causes this problem. Arnd
On Tue, 2023-01-03 at 21:58 +0100, Arnd Bergmann wrote:> On Tue, Jan 3, 2023, at 17:47, Thorsten Glaser wrote: > > Hi, > > > > I noticed a failure of mksh built with klibc on mips64el. > > > > The failing test, on a high level, is this: > > > > :>a > > sleep 2 > > :>b > > test a -nt b > > echo $? > > > > This is supposed to echo 1 (false) because a is not newer than b.[...]> > Does this suffice to begin debugging? > > I don't yet see what the problem is, but it's worth noting that > mips64 is the only architecture using 'unsigned int' for the > st_mtime field, while everything else uses 'long'. There is > a good chance that the different type causes this problem.Right, and klibc's definition of struct stat for mips64 seems to have been copied from mips and edited without fixing that difference. Thorsten: I've attached a patch which I tested briefly in QEMU. Let me know if this works for you. Arnd: I haven't looked at Y2038-proofing klibc yet, but when I do: is there a simple upgrade path for stat() or will I need to wrap statx()? Ben. -- Ben Hutchings The Peter principle: In a hierarchy, every employee tends to rise to their level of incompetence. -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-klibc-mips64-Fix-struct-stat-layout.patch Type: text/x-patch Size: 1249 bytes Desc: not available URL: <https://lists.zytor.com/archives/klibc/attachments/20230104/16e12172/attachment.bin> -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: This is a digitally signed message part URL: <https://lists.zytor.com/archives/klibc/attachments/20230104/16e12172/attachment.sig>