Hi,
This is v2, thank you Thomas for the reply. This version hopefully
addresses the comments you made. I also added some rough tests for the
limits.
Add stdint.h and moved the relevant definitions from std.h. Also added
macros for limits and *_least_* types. Adds tests for the integer
limits.
---
tools/include/nolibc/Makefile | 4 +-
tools/include/nolibc/std.h | 15 +--
tools/include/nolibc/stdint.h | 84 +++++++++++++
tools/testing/selftests/nolibc/nolibc-test.c | 125 ++++++++++++-------
4 files changed, 164 insertions(+), 64 deletions(-)
create mode 100644 tools/include/nolibc/stdint.h
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index cfd06764b5ae..ec57d3932506 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -25,8 +25,8 @@ endif
nolibc_arch := $(patsubst arm64,aarch64,$(ARCH))
arch_file := arch-$(nolibc_arch).h
-all_files := ctype.h errno.h nolibc.h signal.h std.h stdio.h stdlib.h string.h \
- sys.h time.h types.h unistd.h
+all_files := ctype.h errno.h nolibc.h signal.h std.h stdint.h stdio.h stdlib.h \
+ string.h sys.h time.h types.h unistd.h
# install all headers needed to support a bare-metal compiler
all: headers
diff --git a/tools/include/nolibc/std.h b/tools/include/nolibc/std.h
index 1747ae125392..933bc0be7e1c 100644
--- a/tools/include/nolibc/std.h
+++ b/tools/include/nolibc/std.h
@@ -18,20 +18,7 @@
#define NULL ((void *)0)
#endif
-/* stdint types */
-typedef unsigned char uint8_t;
-typedef signed char int8_t;
-typedef unsigned short uint16_t;
-typedef signed short int16_t;
-typedef unsigned int uint32_t;
-typedef signed int int32_t;
-typedef unsigned long long uint64_t;
-typedef signed long long int64_t;
-typedef unsigned long size_t;
-typedef signed long ssize_t;
-typedef unsigned long uintptr_t;
-typedef signed long intptr_t;
-typedef signed long ptrdiff_t;
+#include "stdint.h"
/* those are commonly provided by sys/types.h */
typedef unsigned int dev_t;
diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
new file mode 100644
index 000000000000..a545cb9f6fe8
--- /dev/null
+++ b/tools/include/nolibc/stdint.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+/*
+ * Standard definitions and types for NOLIBC
+ * Copyright (C) 2023 Vincent Dagonneau <[email protected]>
+ */
+
+#ifndef _NOLIBC_STDINT_H
+#define _NOLIBC_STDINT_H
+
+typedef unsigned char uint8_t;
+typedef signed char int8_t;
+typedef unsigned short uint16_t;
+typedef signed short int16_t;
+typedef unsigned int uint32_t;
+typedef signed int int32_t;
+typedef unsigned long long uint64_t;
+typedef signed long long int64_t;
+typedef unsigned long size_t;
+typedef signed long ssize_t;
+typedef unsigned long uintptr_t;
+typedef signed long intptr_t;
+typedef signed long ptrdiff_t;
+
+typedef int8_t int_least8_t;
+typedef uint8_t uint_least8_t;
+typedef int16_t int_least16_t;
+typedef uint16_t uint_least16_t;
+typedef int32_t int_least32_t;
+typedef uint32_t uint_least32_t;
+typedef int64_t int_least64_t;
+typedef uint64_t uint_least64_t;
+
+typedef int64_t intmax_t;
+typedef uint64_t uintmax_t;
+
+/* limits of integral types */
+
+#define INT8_MIN (-128)
+#define INT16_MIN (-32767-1)
+#define INT32_MIN (-2147483647-1)
+#define INT64_MIN (-9223372036854775807-1)
+
+#define INT8_MAX (127)
+#define INT16_MAX (32767)
+#define INT32_MAX (2147483647)
+#define INT64_MAX (9223372036854775807)
+
+#define UINT8_MAX (255)
+#define UINT16_MAX (65535)
+#define UINT32_MAX (4294967295U)
+#define UINT64_MAX (18446744073709551615)
+
+#define INT_LEAST8_MIN INT8_MIN
+#define INT_LEAST16_MIN INT16_MIN
+#define INT_LEAST32_MIN INT32_MIN
+#define INT_LEAST64_MIN INT64_MIN
+
+#define INT_LEAST8_MAX INT8_MAX
+#define INT_LEAST16_MAX INT16_MAX
+#define INT_LEAST32_MAX INT32_MAX
+#define INT_LEAST64_MAX INT64_MAX
+
+#define UINT_LEAST8_MAX UINT8_MAX
+#define UINT_LEAST16_MAX UINT16_MAX
+#define UINT_LEAST32_MAX UINT32_MAX
+#define UINT_LEAST64_MAX UINT64_MAX
+
+#define SIZE_MAX UINT64_MAX
+
+#if __WORDSIZE == 64
+ #define INTPTR_MIN INT64_MIN
+ #define INTPTR_MAX INT64_MAX
+ #define UINTPTR_MAX UINT64_MAX
+ #define PTRDIFF_MIN INT64_MIN
+ #define PTRDIFF_MAX INT64_MAX
+#else
+ #define INTPTR_MIN INT32_MIN
+ #define INTPTR_MAX INT32_MAX
+ #define UINTPTR_MAX UINT32_MAX
+ #define PTRDIFF_MIN INT32_MIN
+ #define PTRDIFF_MAX INT32_MAX
+#endif /* __WORDSIZE == 64 */
+
+#endif /* _NOLIBC_STDINT_H */
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index f14f5076fb6d..0c7698d2a4b7 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -130,111 +130,111 @@ static int pad_spc(int llen, int cnt, const char *fmt, ...)
*/
#define EXPECT_ZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_zr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_zr(expr, llen); } while (0)
static int expect_zr(int expr, int llen)
{
int ret = !(expr == 0);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_NZ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_nz(expr, llen; } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_nz(expr, llen; } while (0)
static int expect_nz(int expr, int llen)
{
int ret = !(expr != 0);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_EQ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_eq(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_eq(expr, llen, val); } while (0)
-static int expect_eq(int expr, int llen, int val)
+static int expect_eq(uint64_t expr, int llen, uint64_t val)
{
int ret = !(expr == val);
- llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ llen += printf(" = %lld ", expr);
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_NE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ne(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ne(expr, llen, val); } while (0)
static int expect_ne(int expr, int llen, int val)
{
int ret = !(expr != val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_GE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ge(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ge(expr, llen, val); } while (0)
static int expect_ge(int expr, int llen, int val)
{
int ret = !(expr >= val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_GT(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_gt(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_gt(expr, llen, val); } while (0)
static int expect_gt(int expr, int llen, int val)
{
int ret = !(expr > val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_LE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_le(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_le(expr, llen, val); } while (0)
static int expect_le(int expr, int llen, int val)
{
int ret = !(expr <= val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_LT(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_lt(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_lt(expr, llen, val); } while (0)
static int expect_lt(int expr, int llen, int val)
{
int ret = !(expr < val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_SYSZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syszr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syszr(expr, llen); } while (0)
static int expect_syszr(int expr, int llen)
{
@@ -243,17 +243,17 @@ static int expect_syszr(int expr, int llen)
if (expr) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSEQ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syseq(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syseq(expr, llen, val); } while (0)
static int expect_syseq(int expr, int llen, int val)
{
@@ -262,17 +262,17 @@ static int expect_syseq(int expr, int llen, int val)
if (expr != val) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSNE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_sysne(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_sysne(expr, llen, val); } while (0)
static int expect_sysne(int expr, int llen, int val)
{
@@ -281,17 +281,17 @@ static int expect_sysne(int expr, int llen, int val)
if (expr == val) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSER(cond, expr, expret, experr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syserr(expr, expret, experr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syserr(expr, expret, experr, llen); } while (0)
static int expect_syserr(int expr, int expret, int experr, int llen)
{
@@ -302,16 +302,16 @@ static int expect_syserr(int expr, int expret, int experr, int llen)
if (expr != expret || _errno != experr) {
ret = 1;
llen += printf(" != (%d %s) ", expret, errorname(experr));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_PTRZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ptrzr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ptrzr(expr, llen); } while (0)
static int expect_ptrzr(const void *expr, int llen)
{
@@ -320,16 +320,16 @@ static int expect_ptrzr(const void *expr, int llen)
llen += printf(" = <%p> ", expr);
if (expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_PTRNZ(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ptrnz(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ptrnz(expr, llen); } while (0)
static int expect_ptrnz(const void *expr, int llen)
{
@@ -338,16 +338,16 @@ static int expect_ptrnz(const void *expr, int llen)
llen += printf(" = <%p> ", expr);
if (!expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strzr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strzr(expr, llen); } while (0)
static int expect_strzr(const char *expr, int llen)
{
@@ -356,16 +356,16 @@ static int expect_strzr(const char *expr, int llen)
llen += printf(" = <%s> ", expr);
if (expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRNZ(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strnz(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strnz(expr, llen); } while (0)
static int expect_strnz(const char *expr, int llen)
{
@@ -374,16 +374,16 @@ static int expect_strnz(const char *expr, int llen)
llen += printf(" = <%s> ", expr);
if (!expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STREQ(cond, expr, cmp) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_streq(expr, llen, cmp); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_streq(expr, llen, cmp); } while (0)
static int expect_streq(const char *expr, int llen, const char *cmp)
{
@@ -392,16 +392,16 @@ static int expect_streq(const char *expr, int llen, const char *cmp)
llen += printf(" = <%s> ", expr);
if (strcmp(expr, cmp) != 0) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRNE(cond, expr, cmp) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strne(expr, llen, cmp); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strne(expr, llen, cmp); } while (0)
static int expect_strne(const char *expr, int llen, const char *cmp)
{
@@ -410,9 +410,9 @@ static int expect_strne(const char *expr, int llen, const char *cmp)
llen += printf(" = <%s> ", expr);
if (strcmp(expr, cmp) == 0) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
@@ -531,6 +531,35 @@ int run_syscall(int min, int max)
CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
+ CASE_TEST(limit_int8_max); EXPECT_EQ(1, INT8_MAX, (int8_t) 0x7f); break;
+ CASE_TEST(limit_int8_min); EXPECT_EQ(1, INT8_MIN, (int8_t) 0x80); break;
+ CASE_TEST(limit_uint8_max); EXPECT_EQ(1, UINT8_MAX, (uint8_t) 0xff); break;
+ CASE_TEST(limit_int16_max); EXPECT_EQ(1, INT16_MAX, (int16_t) 0x7fff); break;
+ CASE_TEST(limit_int16_min); EXPECT_EQ(1, INT16_MIN, (int16_t) 0x8000); break;
+ CASE_TEST(limit_uint16_max); EXPECT_EQ(1, UINT16_MAX, (uint16_t) 0xffff); break;
+ CASE_TEST(limit_int32_max); EXPECT_EQ(1, INT32_MAX, (int32_t) 0x7fffffff); break;
+ CASE_TEST(limit_int32_min); EXPECT_EQ(1, INT32_MIN, (int32_t) 0x80000000); break;
+ CASE_TEST(limit_uint32_max); EXPECT_EQ(1, UINT32_MAX, (uint32_t) 0xffffffff); break;
+ CASE_TEST(limit_int64_max); EXPECT_EQ(1, INT64_MAX, (int64_t) 0x7fffffffffffffff); break;
+ CASE_TEST(limit_int64_min); EXPECT_EQ(1, INT64_MIN, (int64_t) 0x8000000000000000); break;
+ CASE_TEST(limit_uint64_max); EXPECT_EQ(1, UINT64_MAX, (uint64_t) 0xffffffffffffffff); break;
+ CASE_TEST(limit_int_least8_max); EXPECT_EQ(1, INT_LEAST8_MAX, (int_least8_t) 0x7f); break;
+ CASE_TEST(limit_int_least8_min); EXPECT_EQ(1, INT_LEAST8_MIN, (int_least8_t) 0x80); break;
+ CASE_TEST(limit_uint_least8_max); EXPECT_EQ(1, UINT_LEAST8_MAX, (uint_least8_t) 0xff); break;
+ CASE_TEST(limit_int_least16_max); EXPECT_EQ(1, INT_LEAST16_MAX, (int_least16_t) 0x7fff); break;
+ CASE_TEST(limit_int_least16_min); EXPECT_EQ(1, INT_LEAST16_MIN, (int_least16_t) 0x8000); break;
+ CASE_TEST(limit_uint_least16_max); EXPECT_EQ(1, UINT_LEAST16_MAX, (uint_least16_t) 0xffff); break;
+ CASE_TEST(limit_int_least32_max); EXPECT_EQ(1, INT_LEAST32_MAX, (int_least32_t) 0x7fffffff); break;
+ CASE_TEST(limit_int_least32_min); EXPECT_EQ(1, INT_LEAST32_MIN, (int_least32_t) 0x80000000); break;
+ CASE_TEST(limit_uint_least32_max); EXPECT_EQ(1, UINT_LEAST32_MAX, (uint_least32_t) 0xffffffff); break;
+ CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffff); break;
+ CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000); break;
+ CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffff); break;
+ CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (void*) 0x8000000000000000); break;
+ CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (void*) 0x7fffffffffffffff); break;
+ CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (void*) 0xffffffffffffffff); break;
+ CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (void*) 0x8000000000000000); break;
+ CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (void*) 0x7fffffffffffffff); break;
case __LINE__:
return ret; /* must be last */
/* note: do not set any defaults so as to permit holes above */
--
2.39.1
Hi Vincent,
On Thu, Feb 02, 2023 at 03:11:01PM -0500, Vincent Dagonneau wrote:
> Hi,
>
> This is v2, thank you Thomas for the reply. This version hopefully
> addresses the comments you made. I also added some rough tests for the
> limits.
>
> Add stdint.h and moved the relevant definitions from std.h. Also added
> macros for limits and *_least_* types. Adds tests for the integer
> limits.
First, thanks for this work. In order to respond your initial question,
yes there's definitely interest in having something like this to further
ease portability, even if we all know that programs that can be built
with nolibc are really tiny and limited.
A few points though:
- for your commit message, you'll need to put a description of what
it does and the reasons for your choices so that it's easier to
figure later when facing the patch during a bisect session or a
git blame.
- you'll also need to append your signed-off-by tag for the patch to
be merged. Some developers purposely don't put it during reviews
to save it from being merged but then it's better to write
"PATCH RFC" in the subject to make it more obvious.
- the history between your changes, if any, should rather be placed
after the "---" that ends the commit message: it will not appear
in the commit message but still passes the info to the reviewers.
- I'm seeing some preparatory changes in nolibc-test.c that are not
directly caused by the patch itself but by its impact (essentially
the width changes), and these ones should be separate so that they
do not pollute the patch and allow reviewers to focus on the
important part of your changes. I even think that you could proceed
in three steps:
- introduce stdint with the new types and values
- enlarge all fields in the selftest to preserve alignment when
dealing with large return values
- add the integer tests to the suite.
That would give you 3 patches. In the last one it would be appreciated
if you at least mention roughly in which condition you've tested it
(native local test, or qemu for arch x/y or real hardware of what arch).
That will help other testers gauge if it's worth running extra tests on
other platforms or not.
This aside, I'm having a few concerns below:
(...)
> +typedef unsigned long size_t;
> +#define INT64_MIN (-9223372036854775807-1)
This one shoul have "LL" appended, otherwise in some operations it will
be considered as an integer and some values can be truncated.
> +#define INT64_MAX (9223372036854775807)
Same for this one.
> +#define UINT8_MAX (255)
> +#define UINT16_MAX (65535)
> +#define UINT32_MAX (4294967295U)
> +#define UINT64_MAX (18446744073709551615)
This one is missing ULL for the same reasons. Look for example at the
nasty impacts on a 32-bit system:
$ gcc-5.5.0 -xc -Os -fomit-frame-pointer -o /dev/stdout -S - <<< "unsigned x() { return ((18446744073709551615) << 1) >> 40; }" | grep movl
<stdin>: In function 'x':
<stdin>:1:25: warning: integer constant is so large that it is unsigned
movl $33554431, %eax
$ gcc-5.5.0 -xc -Os -fomit-frame-pointer -o /dev/stdout -S - <<< "unsigned x() { return ((18446744073709551615ULL) << 1) >> 40; }" | grep movl
movl $16777215, %eax
See ? When there's no variable around to help figure the type, the default
type will be "int" for the intermediary operations and some calculations
will be wrong without the proper suffix.
> +#define SIZE_MAX UINT64_MAX
This one is not correct, it must follow the word size since it's defined
as an unsigned long (i.e. same as UINTPTR_MAX later).
And below please also add the LL/ULL as needed to the hex values you're
testing to match the expected signedness and size:
> @@ -531,6 +531,35 @@ int run_syscall(int min, int max)
> CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
> CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
> CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
> + CASE_TEST(limit_int8_max); EXPECT_EQ(1, INT8_MAX, (int8_t) 0x7f); break;
> + CASE_TEST(limit_int8_min); EXPECT_EQ(1, INT8_MIN, (int8_t) 0x80); break;
> + CASE_TEST(limit_uint8_max); EXPECT_EQ(1, UINT8_MAX, (uint8_t) 0xff); break;
> + CASE_TEST(limit_int16_max); EXPECT_EQ(1, INT16_MAX, (int16_t) 0x7fff); break;
> + CASE_TEST(limit_int16_min); EXPECT_EQ(1, INT16_MIN, (int16_t) 0x8000); break;
> + CASE_TEST(limit_uint16_max); EXPECT_EQ(1, UINT16_MAX, (uint16_t) 0xffff); break;
> + CASE_TEST(limit_int32_max); EXPECT_EQ(1, INT32_MAX, (int32_t) 0x7fffffff); break;
> + CASE_TEST(limit_int32_min); EXPECT_EQ(1, INT32_MIN, (int32_t) 0x80000000); break;
> + CASE_TEST(limit_uint32_max); EXPECT_EQ(1, UINT32_MAX, (uint32_t) 0xffffffff); break;
> + CASE_TEST(limit_int64_max); EXPECT_EQ(1, INT64_MAX, (int64_t) 0x7fffffffffffffff); break;
> + CASE_TEST(limit_int64_min); EXPECT_EQ(1, INT64_MIN, (int64_t) 0x8000000000000000); break;
> + CASE_TEST(limit_uint64_max); EXPECT_EQ(1, UINT64_MAX, (uint64_t) 0xffffffffffffffff); break;
> + CASE_TEST(limit_int_least8_max); EXPECT_EQ(1, INT_LEAST8_MAX, (int_least8_t) 0x7f); break;
> + CASE_TEST(limit_int_least8_min); EXPECT_EQ(1, INT_LEAST8_MIN, (int_least8_t) 0x80); break;
> + CASE_TEST(limit_uint_least8_max); EXPECT_EQ(1, UINT_LEAST8_MAX, (uint_least8_t) 0xff); break;
> + CASE_TEST(limit_int_least16_max); EXPECT_EQ(1, INT_LEAST16_MAX, (int_least16_t) 0x7fff); break;
> + CASE_TEST(limit_int_least16_min); EXPECT_EQ(1, INT_LEAST16_MIN, (int_least16_t) 0x8000); break;
> + CASE_TEST(limit_uint_least16_max); EXPECT_EQ(1, UINT_LEAST16_MAX, (uint_least16_t) 0xffff); break;
> + CASE_TEST(limit_int_least32_max); EXPECT_EQ(1, INT_LEAST32_MAX, (int_least32_t) 0x7fffffff); break;
> + CASE_TEST(limit_int_least32_min); EXPECT_EQ(1, INT_LEAST32_MIN, (int_least32_t) 0x80000000); break;
> + CASE_TEST(limit_uint_least32_max); EXPECT_EQ(1, UINT_LEAST32_MAX, (uint_least32_t) 0xffffffff); break;
> + CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffff); break;
> + CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000); break;
> + CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffff); break;
> + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (void*) 0x8000000000000000); break;
> + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (void*) 0x7fffffffffffffff); break;
> + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (void*) 0xffffffffffffffff); break;
> + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (void*) 0x8000000000000000); break;
> + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (void*) 0x7fffffffffffffff); break;
Other than that it looks correct.
If you're having doubts, please run it in i386 mode. For this, please have
a look at the thread surrounding this message, as we had a very similar
discussion recently:
https://lore.kernel.org/lkml/[email protected]/
It will show how to run the test for other architectures under qemu
without having to rebuild a whole kernel. For what you're doing it's
a perfect example where it makes sense.
I would also suggest always trying arm (has unsigned chars, could
possibly catch a mistake) and mips (big endian though should not
be affected by your changes, but it's a good habit to develop).
Thanks!
Willy
Hi Willy,
Thank you for the extensive message! I'm already working on a new version including all the feedback you gave.
On Thu, Feb 2, 2023, at 16:46, Willy Tarreau wrote:
> Hi Vincent,
>
> On Thu, Feb 02, 2023 at 03:11:01PM -0500, Vincent Dagonneau wrote:
>> Hi,
>>
>> This is v2, thank you Thomas for the reply. This version hopefully
>> addresses the comments you made. I also added some rough tests for the
>> limits.
>>
>> Add stdint.h and moved the relevant definitions from std.h. Also added
>> macros for limits and *_least_* types. Adds tests for the integer
>> limits.
>
> First, thanks for this work. In order to respond your initial question,
> yes there's definitely interest in having something like this to further
> ease portability, even if we all know that programs that can be built
> with nolibc are really tiny and limited.
>
> A few points though:
> - for your commit message, you'll need to put a description of what
> it does and the reasons for your choices so that it's easier to
> figure later when facing the patch during a bisect session or a
> git blame.
>
Ok, on it.
> - you'll also need to append your signed-off-by tag for the patch to
> be merged. Some developers purposely don't put it during reviews
> to save it from being merged but then it's better to write
> "PATCH RFC" in the subject to make it more obvious.
>
Ok, I'll had the signed-off-by.
> - the history between your changes, if any, should rather be placed
> after the "---" that ends the commit message: it will not appear
> in the commit message but still passes the info to the reviewers.
>
Ok.
> - I'm seeing some preparatory changes in nolibc-test.c that are not
> directly caused by the patch itself but by its impact (essentially
> the width changes), and these ones should be separate so that they
> do not pollute the patch and allow reviewers to focus on the
> important part of your changes. I even think that you could proceed
> in three steps:
> - introduce stdint with the new types and values
> - enlarge all fields in the selftest to preserve alignment when
> dealing with large return values
> - add the integer tests to the suite.
>
I'm thinking of four steps:
- move the existing types to stdint.h, it should compile just fine and would not be adding anything
- add the new stuff to stdint.h (*_least_* types + limit macros)
- enlarge the fields in the test file
- add the tests themselves
Would it work for you?
> That would give you 3 patches. In the last one it would be appreciated
> if you at least mention roughly in which condition you've tested it
> (native local test, or qemu for arch x/y or real hardware of what arch).
> That will help other testers gauge if it's worth running extra tests on
> other platforms or not.
>
> This aside, I'm having a few concerns below:
>
> (...)
>> +typedef unsigned long size_t;
>> +#define INT64_MIN (-9223372036854775807-1)
>
> This one shoul have "LL" appended, otherwise in some operations it will
> be considered as an integer and some values can be truncated.
>
>> +#define INT64_MAX (9223372036854775807)
>
> Same for this one.
>
>> +#define UINT8_MAX (255)
>> +#define UINT16_MAX (65535)
>> +#define UINT32_MAX (4294967295U)
>> +#define UINT64_MAX (18446744073709551615)
>
> This one is missing ULL for the same reasons. Look for example at the
> nasty impacts on a 32-bit system:
>
> $ gcc-5.5.0 -xc -Os -fomit-frame-pointer -o /dev/stdout -S - <<<
> "unsigned x() { return ((18446744073709551615) << 1) >> 40; }" | grep
> movl
> <stdin>: In function 'x':
> <stdin>:1:25: warning: integer constant is so large that it is unsigned
> movl $33554431, %eax
>
> $ gcc-5.5.0 -xc -Os -fomit-frame-pointer -o /dev/stdout -S - <<<
> "unsigned x() { return ((18446744073709551615ULL) << 1) >> 40; }" |
> grep movl
> movl $16777215, %eax
>
> See ? When there's no variable around to help figure the type, the default
> type will be "int" for the intermediary operations and some calculations
> will be wrong without the proper suffix.
>
Thank you for the detailed explanation, it's fixed!
>> +#define SIZE_MAX UINT64_MAX
>
> This one is not correct, it must follow the word size since it's defined
> as an unsigned long (i.e. same as UINTPTR_MAX later).
>
This one is a trick one I think. I've been using https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html as a reference for this patch and it says SIZE_MAX > 65535 (implementation defined). I guess in this case I should follow include/uapi/asm-generic/posix_types.h ?
> And below please also add the LL/ULL as needed to the hex values you're
> testing to match the expected signedness and size:
>
>> @@ -531,6 +531,35 @@ int run_syscall(int min, int max)
>> CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
>> CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
>> CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
>> + CASE_TEST(limit_int8_max); EXPECT_EQ(1, INT8_MAX, (int8_t) 0x7f); break;
>> + CASE_TEST(limit_int8_min); EXPECT_EQ(1, INT8_MIN, (int8_t) 0x80); break;
>> + CASE_TEST(limit_uint8_max); EXPECT_EQ(1, UINT8_MAX, (uint8_t) 0xff); break;
>> + CASE_TEST(limit_int16_max); EXPECT_EQ(1, INT16_MAX, (int16_t) 0x7fff); break;
>> + CASE_TEST(limit_int16_min); EXPECT_EQ(1, INT16_MIN, (int16_t) 0x8000); break;
>> + CASE_TEST(limit_uint16_max); EXPECT_EQ(1, UINT16_MAX, (uint16_t) 0xffff); break;
>> + CASE_TEST(limit_int32_max); EXPECT_EQ(1, INT32_MAX, (int32_t) 0x7fffffff); break;
>> + CASE_TEST(limit_int32_min); EXPECT_EQ(1, INT32_MIN, (int32_t) 0x80000000); break;
>> + CASE_TEST(limit_uint32_max); EXPECT_EQ(1, UINT32_MAX, (uint32_t) 0xffffffff); break;
>> + CASE_TEST(limit_int64_max); EXPECT_EQ(1, INT64_MAX, (int64_t) 0x7fffffffffffffff); break;
>> + CASE_TEST(limit_int64_min); EXPECT_EQ(1, INT64_MIN, (int64_t) 0x8000000000000000); break;
>> + CASE_TEST(limit_uint64_max); EXPECT_EQ(1, UINT64_MAX, (uint64_t) 0xffffffffffffffff); break;
>> + CASE_TEST(limit_int_least8_max); EXPECT_EQ(1, INT_LEAST8_MAX, (int_least8_t) 0x7f); break;
>> + CASE_TEST(limit_int_least8_min); EXPECT_EQ(1, INT_LEAST8_MIN, (int_least8_t) 0x80); break;
>> + CASE_TEST(limit_uint_least8_max); EXPECT_EQ(1, UINT_LEAST8_MAX, (uint_least8_t) 0xff); break;
>> + CASE_TEST(limit_int_least16_max); EXPECT_EQ(1, INT_LEAST16_MAX, (int_least16_t) 0x7fff); break;
>> + CASE_TEST(limit_int_least16_min); EXPECT_EQ(1, INT_LEAST16_MIN, (int_least16_t) 0x8000); break;
>> + CASE_TEST(limit_uint_least16_max); EXPECT_EQ(1, UINT_LEAST16_MAX, (uint_least16_t) 0xffff); break;
>> + CASE_TEST(limit_int_least32_max); EXPECT_EQ(1, INT_LEAST32_MAX, (int_least32_t) 0x7fffffff); break;
>> + CASE_TEST(limit_int_least32_min); EXPECT_EQ(1, INT_LEAST32_MIN, (int_least32_t) 0x80000000); break;
>> + CASE_TEST(limit_uint_least32_max); EXPECT_EQ(1, UINT_LEAST32_MAX, (uint_least32_t) 0xffffffff); break;
>> + CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffff); break;
>> + CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000); break;
>> + CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffff); break;
>> + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (void*) 0x8000000000000000); break;
>> + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (void*) 0x7fffffffffffffff); break;
>> + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (void*) 0xffffffffffffffff); break;
>> + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (void*) 0x8000000000000000); break;
>> + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (void*) 0x7fffffffffffffff); break;
>
Thanks, done!
> Other than that it looks correct.
>
> If you're having doubts, please run it in i386 mode. For this, please have
> a look at the thread surrounding this message, as we had a very similar
> discussion recently:
>
>
> https://lore.kernel.org/lkml/[email protected]/
>
> It will show how to run the test for other architectures under qemu
> without having to rebuild a whole kernel. For what you're doing it's
> a perfect example where it makes sense.
>
> I would also suggest always trying arm (has unsigned chars, could
> possibly catch a mistake) and mips (big endian though should not
> be affected by your changes, but it's a good habit to develop).
>
Yes, as soon as I have some workable patch I'll test it on other arch. Most of it will be on qemu though, I might be able to test on x86_64 and arm64 hardware but that is all I have.
> Thanks!
> Willy
Thank you for the review, I do appreciate the time you've spent on the response. I'll get a new version out soon.
Vincent.
On Thu, Feb 02, 2023 at 06:29:59PM -0500, Vincent Dagonneau wrote:
> > - I'm seeing some preparatory changes in nolibc-test.c that are not
> > directly caused by the patch itself but by its impact (essentially
> > the width changes), and these ones should be separate so that they
> > do not pollute the patch and allow reviewers to focus on the
> > important part of your changes. I even think that you could proceed
> > in three steps:
> > - introduce stdint with the new types and values
> > - enlarge all fields in the selftest to preserve alignment when
> > dealing with large return values
> > - add the integer tests to the suite.
> >
>
> I'm thinking of four steps:
> - move the existing types to stdint.h, it should compile just fine and would not be adding anything
> - add the new stuff to stdint.h (*_least_* types + limit macros)
> - enlarge the fields in the test file
> - add the tests themselves
>
> Would it work for you?
You're right, it's even better. I'm happy that you got the principle :-)
> >> +#define SIZE_MAX UINT64_MAX
> >
> > This one is not correct, it must follow the word size since it's defined
> > as an unsigned long (i.e. same as UINTPTR_MAX later).
> >
>
> This one is a trick one I think. I've been using
> https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html as a
> reference for this patch and it says SIZE_MAX > 65535 (implementation
> defined).
In fact it says, that these are the minimum ranges an implementation must
support.
> I guess in this case I should follow
> include/uapi/asm-generic/posix_types.h ?
Yes, that's the same principle there, indeed.
> > Other than that it looks correct.
> >
> > If you're having doubts, please run it in i386 mode. For this, please have
> > a look at the thread surrounding this message, as we had a very similar
> > discussion recently:
> >
> >
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > It will show how to run the test for other architectures under qemu
> > without having to rebuild a whole kernel. For what you're doing it's
> > a perfect example where it makes sense.
> >
> > I would also suggest always trying arm (has unsigned chars, could
> > possibly catch a mistake) and mips (big endian though should not
> > be affected by your changes, but it's a good habit to develop).
> >
>
> Yes, as soon as I have some workable patch I'll test it on other arch. Most
> of it will be on qemu though, I might be able to test on x86_64 and arm64
> hardware but that is all I have.
It's fine to run on qemu for this because what you're testing is that
you're using the right combinations of types and ranges on the different
supported platforms, which means verifying that each compiler will produce
the code you're expecting. There's no hardware dependency nor any need to
verify something related to the kernel itself.
> > Thanks!
> > Willy
>
> Thank you for the review, I do appreciate the time you've spent on the
> response. I'll get a new version out soon.
You're welcome!
Willy