| Message ID | 20230530-nolibc-fast64-v1-1-883dea6bc666@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: ensure fast64 integer types have 64 bits | expand |
On Tue, May 30, 2023 at 11:18:00AM +0200, Thomas Weißschuh wrote: > On 32bit platforms size_t is not enough to represent [u]int_fast64_t. > > Fixes: 3e9fd4e9a1d5 ("tools/nolibc: add integer types and integer limit macros") > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > Cc: Vincent Dagonneau <v@vda.io> > > Note: We could also fall back to compiler-provided data like: > > __UINT_FAST{8,16,32,64}_{TYPE,MIN,MAX}__ I'm fine with the way you did it. I'm wondering how we managed to miss this one given the tests in place! Now queued, thank you Thomas. Willy
On Sun, Jun 04, 2023 at 12:50:03PM +0200, Willy Tarreau wrote: > On Tue, May 30, 2023 at 11:18:00AM +0200, Thomas Weißschuh wrote: > > On 32bit platforms size_t is not enough to represent [u]int_fast64_t. > > > > Fixes: 3e9fd4e9a1d5 ("tools/nolibc: add integer types and integer limit macros") > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > --- > > Cc: Vincent Dagonneau <v@vda.io> > > > > Note: We could also fall back to compiler-provided data like: > > > > __UINT_FAST{8,16,32,64}_{TYPE,MIN,MAX}__ > > I'm fine with the way you did it. I'm wondering how we managed to miss > this one given the tests in place! BTW, it failed on 32-bit platforms: 4407 tests passed, 84 skipped, 63 failed $ grep '^linux_arch\|FAIL' test14.out linux_arch=i386 qemu_arch=i386 gcc_arch=i386 52 limit_int_fast64_min = -2147483648 [FAIL] 53 limit_int_fast64_max = 2147483647 [FAIL] 54 limit_uint_fast64_max = 4294967295 [FAIL] The reason is that the constants also have to be adjusted. With the fix below everything works right: --- a/tools/include/nolibc/stdint.h +++ b/tools/include/nolibc/stdint.h @@ -84,17 +84,17 @@ typedef uint64_t uintmax_t; #define INT_FAST8_MIN INT8_MIN #define INT_FAST16_MIN INTPTR_MIN #define INT_FAST32_MIN INTPTR_MIN -#define INT_FAST64_MIN INTPTR_MIN +#define INT_FAST64_MIN INT64_MIN #define INT_FAST8_MAX INT8_MAX #define INT_FAST16_MAX INTPTR_MAX #define INT_FAST32_MAX INTPTR_MAX -#define INT_FAST64_MAX INTPTR_MAX +#define INT_FAST64_MAX INT64_MAX #define UINT_FAST8_MAX UINT8_MAX #define UINT_FAST16_MAX SIZE_MAX #define UINT_FAST32_MAX SIZE_MAX -#define UINT_FAST64_MAX SIZE_MAX +#define UINT_FAST64_MAX UINT64_MAX #ifndef INT_MIN #define INT_MIN (-__INT_MAX__ - 1) 4470 tests passed, 84 skipped, 0 failed $ grep '^linux_arch\|fast64' test15.out linux_arch=i386 qemu_arch=i386 gcc_arch=i386 52 limit_int_fast64_min = -9223372036854775808 [OK] 53 limit_int_fast64_max = 9223372036854775807 [OK] 54 limit_uint_fast64_max = -1 [OK] If you're fine with it, I'll squash it into your patch. Thanks, Willy
On 2023-06-04 13:59:42+0200, Willy Tarreau wrote: > On Sun, Jun 04, 2023 at 12:50:03PM +0200, Willy Tarreau wrote: > > On Tue, May 30, 2023 at 11:18:00AM +0200, Thomas Weißschuh wrote: > > > On 32bit platforms size_t is not enough to represent [u]int_fast64_t. > > > > > > Fixes: 3e9fd4e9a1d5 ("tools/nolibc: add integer types and integer limit macros") > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > > --- > > > Cc: Vincent Dagonneau <v@vda.io> > > > > > > Note: We could also fall back to compiler-provided data like: > > > > > > __UINT_FAST{8,16,32,64}_{TYPE,MIN,MAX}__ > > > > I'm fine with the way you did it. I'm wondering how we managed to miss > > this one given the tests in place! > > BTW, it failed on 32-bit platforms: > > 4407 tests passed, 84 skipped, 63 failed > $ grep '^linux_arch\|FAIL' test14.out > linux_arch=i386 qemu_arch=i386 gcc_arch=i386 > 52 limit_int_fast64_min = -2147483648 [FAIL] > 53 limit_int_fast64_max = 2147483647 [FAIL] > 54 limit_uint_fast64_max = 4294967295 [FAIL] > > The reason is that the constants also have to be adjusted. With the fix > below everything works right: > > --- a/tools/include/nolibc/stdint.h > +++ b/tools/include/nolibc/stdint.h > @@ -84,17 +84,17 @@ typedef uint64_t uintmax_t; > #define INT_FAST8_MIN INT8_MIN > #define INT_FAST16_MIN INTPTR_MIN > #define INT_FAST32_MIN INTPTR_MIN > -#define INT_FAST64_MIN INTPTR_MIN > +#define INT_FAST64_MIN INT64_MIN > > #define INT_FAST8_MAX INT8_MAX > #define INT_FAST16_MAX INTPTR_MAX > #define INT_FAST32_MAX INTPTR_MAX > -#define INT_FAST64_MAX INTPTR_MAX > +#define INT_FAST64_MAX INT64_MAX > > #define UINT_FAST8_MAX UINT8_MAX > #define UINT_FAST16_MAX SIZE_MAX > #define UINT_FAST32_MAX SIZE_MAX > -#define UINT_FAST64_MAX SIZE_MAX > +#define UINT_FAST64_MAX UINT64_MAX > > #ifndef INT_MIN > #define INT_MIN (-__INT_MAX__ - 1) > > > 4470 tests passed, 84 skipped, 0 failed > $ grep '^linux_arch\|fast64' test15.out > linux_arch=i386 qemu_arch=i386 gcc_arch=i386 > 52 limit_int_fast64_min = -9223372036854775808 [OK] > 53 limit_int_fast64_max = 9223372036854775807 [OK] > 54 limit_uint_fast64_max = -1 [OK] > > If you're fine with it, I'll squash it into your patch. Yes, please do so. Fitting quote: "I'm wondering how we managed to miss this one given the tests in place!"
diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h index c1ce4f5e0603..3fc418cfc3d7 100644 --- a/tools/include/nolibc/stdint.h +++ b/tools/include/nolibc/stdint.h @@ -36,8 +36,8 @@ typedef ssize_t int_fast16_t; typedef size_t uint_fast16_t; typedef ssize_t int_fast32_t; typedef size_t uint_fast32_t; -typedef ssize_t int_fast64_t; -typedef size_t uint_fast64_t; +typedef int64_t int_fast64_t; +typedef uint64_t uint_fast64_t; typedef int64_t intmax_t; typedef uint64_t uintmax_t; diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c index 7de46305f419..65be0317d184 100644 --- a/tools/testing/selftests/nolibc/nolibc-test.c +++ b/tools/testing/selftests/nolibc/nolibc-test.c @@ -696,9 +696,9 @@ int run_stdlib(int min, int max) CASE_TEST(limit_int_fast32_min); EXPECT_EQ(1, INT_FAST32_MIN, (int_fast32_t) INTPTR_MIN); break; CASE_TEST(limit_int_fast32_max); EXPECT_EQ(1, INT_FAST32_MAX, (int_fast32_t) INTPTR_MAX); break; CASE_TEST(limit_uint_fast32_max); EXPECT_EQ(1, UINT_FAST32_MAX, (uint_fast32_t) UINTPTR_MAX); break; - CASE_TEST(limit_int_fast64_min); EXPECT_EQ(1, INT_FAST64_MIN, (int_fast64_t) INTPTR_MIN); break; - CASE_TEST(limit_int_fast64_max); EXPECT_EQ(1, INT_FAST64_MAX, (int_fast64_t) INTPTR_MAX); break; - CASE_TEST(limit_uint_fast64_max); EXPECT_EQ(1, UINT_FAST64_MAX, (uint_fast64_t) UINTPTR_MAX); break; + CASE_TEST(limit_int_fast64_min); EXPECT_EQ(1, INT_FAST64_MIN, (int_fast64_t) INT64_MIN); break; + CASE_TEST(limit_int_fast64_max); EXPECT_EQ(1, INT_FAST64_MAX, (int_fast64_t) INT64_MAX); break; + CASE_TEST(limit_uint_fast64_max); EXPECT_EQ(1, UINT_FAST64_MAX, (uint_fast64_t) UINT64_MAX); break; #if __SIZEOF_LONG__ == 8 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;
On 32bit platforms size_t is not enough to represent [u]int_fast64_t. Fixes: 3e9fd4e9a1d5 ("tools/nolibc: add integer types and integer limit macros") Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- Cc: Vincent Dagonneau <v@vda.io> Note: We could also fall back to compiler-provided data like: __UINT_FAST{8,16,32,64}_{TYPE,MIN,MAX}__ --- tools/include/nolibc/stdint.h | 4 ++-- tools/testing/selftests/nolibc/nolibc-test.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) --- base-commit: 5b21219d67d3483144d10332709d0c04f733ab93 change-id: 20230530-nolibc-fast64-8777cdbc7e01 Best regards,