Re: [PATCH v4 0/4] tools/nolibc: Adding stdint.h, more integer types and tests

Willy Tarreau
2023-02-12T10:47:08Z

Hi Vincent,

On Fri, Feb 10, 2023 at 08:03:02AM -0500, Vincent Dagonneau wrote:
> > Thanks Vincent. At first glance it looks good. I'll give it a try on
> > all supported archs to make sure we didn't overlook anything and we'll
> > merge it. One tiny comment though, look below:
> >
> >> Vincent Dagonneau (4):
> >> tools/nolibc: Adding stdint.h
> >> tools/nolibc: Adding integer types and integer limit macros
> >> tools/nolibc: Enlarging column width of tests
> >> tools/nolibc: Adds tests for the integer limits in stdint.h
> >
> > I mentioned in the first review that it's generally preferred to use
> > the imperative form rather than present participle on subject lines.
> > This would give:
> >
> > tools/nolibc: Add stdint.h
> > tools/nolibc: Add integer types and integer limit macros
> > tools/nolibc: Enlarge column width of tests
> > tools/nolibc: Add tests for the integer limits in stdint.h
> >
> > I can perform this trivial change locally before merging without asking
> > you to respin a series just for this if that's OK for you. Just let me
> > know.
> >
> > Thanks!
> > Willy
>
> Yep, go ahead!

Done, however I'm seeing the following failures on aarch64/riscv64/s390x:

$ grep -B 200 limit.*FAIL stdint.out | grep '\(limit.*FAIL\|gcc-11.3.0-nolibc.*-O0\)'
/f/tc/nolibc/gcc-11.3.0-nolibc/aarch64*/bin/aarch64*-gcc -g -O0 -g -o nolibc-test \
100 limit_intptr_min = -2147483648 [FAIL]
103 limit_ptrdiff_min = -2147483648 [FAIL]
105 limit_ptrdiff_min = -2147483648 [FAIL]
/f/tc/nolibc/gcc-11.3.0-nolibc/riscv64*/bin/riscv64*-gcc -g -O0 -g -o nolibc-test \
100 limit_intptr_min = -2147483648 [FAIL]
103 limit_ptrdiff_min = -2147483648 [FAIL]
105 limit_ptrdiff_min = -2147483648 [FAIL]
/f/tc/nolibc/gcc-11.3.0-nolibc/s390*/bin/s390*-gcc -g -march=z10 -m64 -O0 -g -o nolibc-test \
100 limit_intptr_min = -2147483648 [FAIL]
103 limit_ptrdiff_min = -2147483648 [FAIL]
105 limit_ptrdiff_min = -2147483648 [FAIL]

It makes me think that the __WORDSIZE==64 condition didn't match there,
I'm investigating. However while looking at this I noticed a mistake in
your patch: in the 32-bit part, limit_ptrdiff_{min,max} were repeated,
and no least64_{min,max} tests were placed, so I sense a copy-paste
mistake though I'm uncertain about the initial intent. If you just want
me to drop the duplicate lines I can easily do it, just let me know. I'll
be back with more info once I figure the reason for these archs not using
__WORDSIZE==64.

#if __WORDSIZE == 64
CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffffLL); break;
CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000LL); break;
CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffffULL); break;
CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x8000000000000000LL); break;
CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffffffffffffLL); break;
CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffffffffffULL); break;
CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x8000000000000000LL); break;
CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffffffffffffLL); break;
CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffffffffffULL); break;
#else
CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break;
CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break;
CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break;
CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break;
#endif /* __WORDSIZE == 64 */

Regards,
Willy