linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc
@ 2022-03-20  9:37 Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 1/6] tools/nolibc: x86-64: Update System V ABI document link Ammar Faizi
                   ` (5 more replies)
  0 siblings, 6 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi

Hi,

This is the v1 of RFC to add dynamic memory allocator support for
nolibc.


## Background

The need to allocate memory dynamically has become a requirement for
the C programming language. Mainly it happens when the allocation size
is determined at runtime. Many other use cases also do it when the
object's lifetime is long-lived and needs to be recycled at runtime.

Currently, the nolibc header doesn't support such a type of allocation.
This series adds it.


## Implementation

Add basic functions to manage dynamic memory allocation:
  - malloc()
  - calloc()
  - realloc()
  - free()

The allocator uses mmap() syscall to allocate the memory and uses
munmap() syscall to free the allocated memory.

The metadata to keep track the length for munmap-ing is simply
defined as a struct below:
```
struct nolibc_heap {
        size_t  len;
        char    user_p[] __attribute__((__aligned__));
};
```
malloc(), realloc() and calloc() return a pointer to `user_p`.


## Add my_syscall6() support for x86 32-bit.

mmap() needs 6 arguments to work with. Not all architectures that
nolibc supports have the my_syscall6() wrapper. This series also
adds my_syscall6() wrapper support for i386.

Notes:

Both Clang and GCC cannot use %ebp in the clobber list and in the "r"
constraint without using -fomit-frame-pointer. To make it always
available for any kind of compilation, the below workaround is
implemented.

For clang (the Assembly statement can't clobber %ebp):
  1) Save the %ebp value to the redzone area -4(%esp).
  2) Load the 6-th argument from memory to %ebp.
  3) Subtract the %esp by 4.
  4) Do the syscall (int $0x80).
  5) Pop %ebp.

For GCC, fortunately it has a #pragma that can force a specific function
to be compiled with -fomit-frame-pointer, so it can use "r"(var) where
var is a variable bound to %ebp.


## Limitation

Currently, for mips and arm arch cannot use these dynamic memory allocator
functions because they're missing the my_syscall6() macro.

[ammarfaizi2: I would love to add the support for them too, but I don't
 have the hardware to play with MIPS and ARM. ]


## Test

The following simple program can be used to test this series:

  https://gist.github.com/ammarfaizi2/db0af6aa0b95a0c7478bce64e349f021


## Patchset Summary

1) Patch 1 is a fix for the System V ABI document link.

2) Patch 2 is a fix to support compile with clang.

3) Patch 3 adds my_syscall6() implementation for i386.

4) Patch 4 adds mmap() and munmap() functions.

5) Patch 5 adds malloc(), calloc(), realloc() and free().

6) Patch 6 adds strdup() and strndup().


Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
Ammar Faizi (6):
  tools/nolibc: x86-64: Update System V ABI document link
  tools/nolibc: Make the entry point not weak for clang
  tools/nolibc: i386: Implement syscall with 6 arguments
  tools/nolibc/sys: Implement `mmap()` and `munmap()`
  tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  tools/include/string: Implement `strdup()` and `strndup()`

 tools/include/nolibc/arch-aarch64.h |  2 +
 tools/include/nolibc/arch-arm.h     |  2 +
 tools/include/nolibc/arch-i386.h    | 66 ++++++++++++++++++++++++
 tools/include/nolibc/arch-mips.h    |  2 +
 tools/include/nolibc/arch-riscv.h   |  2 +
 tools/include/nolibc/arch-x86_64.h  |  4 +-
 tools/include/nolibc/stdlib.h       | 79 +++++++++++++++++++++++++++++
 tools/include/nolibc/string.h       | 68 +++++++++++++++++++++++++
 tools/include/nolibc/sys.h          | 62 ++++++++++++++++++++++
 9 files changed, 286 insertions(+), 1 deletion(-)


base-commit: fda0d5d1b79d8b7032be3d7720a481a9fde91baf
-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 1/6] tools/nolibc: x86-64: Update System V ABI document link
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang Ammar Faizi
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi

The old link no longer works, update it.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/arch-x86_64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index fe517c16cd4d..a7b70ea51b68 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -61,7 +61,7 @@ struct sys_stat_struct {
  *   - see also x86-64 ABI section A.2 AMD64 Linux Kernel Conventions, A.2.1
  *     Calling Conventions.
  *
- * Link x86-64 ABI: https://gitlab.com/x86-psABIs/x86-64-ABI/-/wikis/x86-64-psABI
+ * Link x86-64 ABI: https://gitlab.com/x86-psABIs/x86-64-ABI/-/wikis/home
  *
  */
 
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 1/6] tools/nolibc: x86-64: Update System V ABI document link Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20 19:16   ` Willy Tarreau
  2022-03-20  9:37 ` [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi,
	llvm

Budilig with clang yields the following error:
```
  <inline asm>:3:1: error: _start changed binding to STB_GLOBAL
  .global _start
  ^
  1 error generated.
```
Don't make the entry point weak if we're compiling with clang.

Cc: llvm@lists.linux.dev
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/arch-aarch64.h | 2 ++
 tools/include/nolibc/arch-arm.h     | 2 ++
 tools/include/nolibc/arch-i386.h    | 2 ++
 tools/include/nolibc/arch-mips.h    | 2 ++
 tools/include/nolibc/arch-riscv.h   | 2 ++
 tools/include/nolibc/arch-x86_64.h  | 2 ++
 6 files changed, 12 insertions(+)

diff --git a/tools/include/nolibc/arch-aarch64.h b/tools/include/nolibc/arch-aarch64.h
index 87d9e434820c..5084cd58b429 100644
--- a/tools/include/nolibc/arch-aarch64.h
+++ b/tools/include/nolibc/arch-aarch64.h
@@ -183,7 +183,9 @@ struct sys_stat_struct {
 
 /* startup code */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak _start\n"
+#endif
     ".global _start\n"
     "_start:\n"
     "ldr x0, [sp]\n"              // argc (x0) was in the stack
diff --git a/tools/include/nolibc/arch-arm.h b/tools/include/nolibc/arch-arm.h
index 001a3c8c9ad5..b3f135a615a6 100644
--- a/tools/include/nolibc/arch-arm.h
+++ b/tools/include/nolibc/arch-arm.h
@@ -176,7 +176,9 @@ struct sys_stat_struct {
 
 /* startup code */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak _start\n"
+#endif
     ".global _start\n"
     "_start:\n"
 #if defined(__THUMBEB__) || defined(__THUMBEL__)
diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index d7e4d53325a3..82bf797849ae 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -175,7 +175,9 @@ struct sys_stat_struct {
  *
  */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak _start\n"
+#endif
     ".global _start\n"
     "_start:\n"
     "pop %eax\n"                // argc   (first arg, %eax)
diff --git a/tools/include/nolibc/arch-mips.h b/tools/include/nolibc/arch-mips.h
index c9a6aac87c6d..719d3808614d 100644
--- a/tools/include/nolibc/arch-mips.h
+++ b/tools/include/nolibc/arch-mips.h
@@ -190,7 +190,9 @@ struct sys_stat_struct {
 
 /* startup code, note that it's called __start on MIPS */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak __start\n"
+#endif
     ".set nomips16\n"
     ".global __start\n"
     ".set    noreorder\n"
diff --git a/tools/include/nolibc/arch-riscv.h b/tools/include/nolibc/arch-riscv.h
index bc10b7b5706d..a9704affd7de 100644
--- a/tools/include/nolibc/arch-riscv.h
+++ b/tools/include/nolibc/arch-riscv.h
@@ -184,7 +184,9 @@ struct sys_stat_struct {
 
 /* startup code */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak _start\n"
+#endif
     ".global _start\n"
     "_start:\n"
     ".option push\n"
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h
index a7b70ea51b68..f453f1a05a48 100644
--- a/tools/include/nolibc/arch-x86_64.h
+++ b/tools/include/nolibc/arch-x86_64.h
@@ -198,7 +198,9 @@ struct sys_stat_struct {
  *
  */
 asm(".section .text\n"
+#if !defined(__clang__)
     ".weak _start\n"
+#endif
     ".global _start\n"
     "_start:\n"
     "pop %rdi\n"                // argc   (first arg, %rdi)
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 1/6] tools/nolibc: x86-64: Update System V ABI document link Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20 10:33   ` Alviro Iskandar Setiawan
  2022-03-20 13:10   ` David Laight
  2022-03-20  9:37 ` [RFC PATCH v1 4/6] tools/nolibc/sys: Implement `mmap()` and `munmap()` Ammar Faizi
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi,
	x86, llvm

In i386, the 6th argument of syscall goes in %ebp. However, both Clang
and GCC cannot use %ebp in the clobber list and in the "r" constraint
without using -fomit-frame-pointer. To make it always available for any
kind of compilation, the below workaround is implemented.

For clang (the Assembly statement can't clobber %ebp):
  1) Save the %ebp value to the redzone area -4(%esp).
  2) Load the 6-th argument from memory to %ebp.
  3) Subtract the %esp by 4.
  4) Do the syscall (int $0x80).
  5) Pop %ebp.

For GCC, fortunately it has a #pragma that can force a specific function
to be compiled with -fomit-frame-pointer, so it can always use "r"(var)
where `var` is a variable bound to %ebp.

Cc: x86@kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/arch-i386.h | 64 ++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/tools/include/nolibc/arch-i386.h b/tools/include/nolibc/arch-i386.h
index 82bf797849ae..10de54d4b4d6 100644
--- a/tools/include/nolibc/arch-i386.h
+++ b/tools/include/nolibc/arch-i386.h
@@ -167,6 +167,70 @@ struct sys_stat_struct {
 	_ret;                                                                 \
 })
 
+
+/*
+ * Both Clang and GCC cannot use %ebp in the clobber list and in the "r"
+ * constraint without using -fomit-frame-pointer. To make it always
+ * available for any kind of compilation, the below workaround is
+ * implemented.
+ *
+ * For clang (the Assembly statement can't clobber %ebp):
+ *   1) Save the %ebp value to the redzone area -4(%esp).
+ *   2) Load the 6-th argument from memory to %ebp.
+ *   3) Subtract the %esp by 4.
+ *   4) Do the syscall (int $0x80).
+ *   5) Pop %ebp.
+ *
+ * For GCC, fortunately it has a #pragma that can force a specific function
+ * to be compiled with -fomit-frame-pointer, so it can use "r"(var) where
+ * var is a variable bound to %ebp.
+ *
+ */
+#if defined(__clang__)
+static inline long ____do_syscall6(long eax, long ebx, long ecx, long edx,
+				   long esi, long edi, long ebp)
+{
+	__asm__ volatile (
+		"movl	%%ebp, -4(%%esp)\n\t"
+		"movl	%[arg6], %%ebp\n\t"
+		"subl	$4, %%esp\n\t"
+		"int	$0x80\n\t"
+		"popl	%%ebp\n\t"
+		: "=a"(eax)
+		: "a"(eax), "b"(ebx), "c"(ecx), "d"(edx), "S"(esi), "D"(edi),
+		  [arg6]"m"(ebp)
+		: "memory", "cc"
+	);
+	return eax;
+}
+
+#else /* #if defined(__clang__) */
+#pragma GCC push_options
+#pragma GCC optimize "-fomit-frame-pointer"
+static inline long ____do_syscall6(long eax, long ebx, long ecx, long edx,
+				   long esi, long edi, long ebp)
+{
+	register long __ebp __asm__("ebp") = ebp;
+	__asm__ volatile (
+		"int	$0x80"
+		: "=a"(eax)
+		: "a"(eax), "b"(ebx), "c"(ecx), "d"(edx), "S"(esi), "D"(edi),
+		  "r"(__ebp)
+		: "memory", "cc"
+	);
+	return eax;
+}
+#pragma GCC pop_options
+#endif /* #if defined(__clang__) */
+
+#define my_syscall6(num, arg1, arg2, arg3, arg4, arg5, arg6) (   \
+	____do_syscall6((long)(num), (long)(arg1),               \
+			(long)(arg2), (long)(arg3),              \
+			(long)(arg4), (long)(arg5),              \
+			(long)(arg6))                            \
+)
+
+
 /* startup code */
 /*
  * i386 System V ABI mandates:
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 4/6] tools/nolibc/sys: Implement `mmap()` and `munmap()`
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
                   ` (2 preceding siblings ...)
  2022-03-20  9:37 ` [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
  2022-03-20  9:37 ` [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi
  5 siblings, 0 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi

Implement mmap() and munmap(). Currently, they are only available for
architecures that have my_syscall6 macro. For architectures that don't
have, this function will return -1 with errno set to -ENOSYS (Function
not implemented).

This has been tested on x86 and i386.

Notes for i386:
 1) The common mmap() syscall implementation uses __NR_mmap2 instead
    of __NR_mmap.
 2) The offset must be shifted-right by 12-bit.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/sys.h | 62 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 28437863c63f..76510b19b7cf 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -14,6 +14,7 @@
 #include <asm/unistd.h>
 #include <asm/signal.h>  // for SIGCHLD
 #include <asm/ioctls.h>
+#include <asm/mman.h>
 #include <linux/fs.h>
 #include <linux/loop.h>
 #include <linux/time.h>
@@ -658,6 +659,67 @@ int mknod(const char *path, mode_t mode, dev_t dev)
 	return ret;
 }
 
+#ifndef MAP_SHARED
+#define MAP_SHARED		0x01	/* Share changes */
+#define MAP_PRIVATE		0x02	/* Changes are private */
+#define MAP_SHARED_VALIDATE	0x03	/* share + validate extension flags */
+#endif
+
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+static __attribute__((unused))
+void *sys_mmap(void *addr, size_t length, int prot, int flags, int fd,
+	       off_t offset)
+{
+#ifndef my_syscall6
+	/* Function not implemented. */
+	return -ENOSYS;
+#else
+
+	int n;
+
+#if defined(__i386__)
+	n = __NR_mmap2;
+	offset >>= 12;
+#else
+	n = __NR_mmap;
+#endif
+
+	return (void *)my_syscall6(n, addr, length, prot, flags, fd, offset);
+#endif
+}
+
+static __attribute__((unused))
+void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset)
+{
+	void *ret = sys_mmap(addr, length, prot, flags, fd, offset);
+
+	if ((unsigned long)ret >= -4095UL) {
+		SET_ERRNO(-(long)ret);
+		ret = MAP_FAILED;
+	}
+	return ret;
+}
+
+static __attribute__((unused))
+int sys_munmap(void *addr, size_t length)
+{
+	return my_syscall2(__NR_munmap, addr, length);
+}
+
+static __attribute__((unused))
+int munmap(void *addr, size_t length)
+{
+	int ret = sys_munmap(addr, length);
+
+	if (ret < 0) {
+		SET_ERRNO(-ret);
+		ret = -1;
+	}
+	return ret;
+}
 
 /*
  * int mount(const char *source, const char *target,
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
                   ` (3 preceding siblings ...)
  2022-03-20  9:37 ` [RFC PATCH v1 4/6] tools/nolibc/sys: Implement `mmap()` and `munmap()` Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20 15:50   ` Alviro Iskandar Setiawan
  2022-03-20 16:16   ` Willy Tarreau
  2022-03-20  9:37 ` [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi
  5 siblings, 2 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi

Implement basic dynamic allocator functions. These functions are
currently only available on architectures that have nolibc mmap()
syscall implemented. These are not a super-fast memory allocator,
but at least they can satisfy basic needs for having heap without
libc.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/stdlib.h | 79 +++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/tools/include/nolibc/stdlib.h b/tools/include/nolibc/stdlib.h
index 733105c574ee..13600e73404d 100644
--- a/tools/include/nolibc/stdlib.h
+++ b/tools/include/nolibc/stdlib.h
@@ -10,8 +10,24 @@
 #include "std.h"
 #include "arch.h"
 #include "types.h"
+#include "string.h"
 #include "sys.h"
 
+struct nolibc_heap {
+	size_t	len;
+	char	user_p[] __attribute__((__aligned__));
+};
+
+#ifndef offsetof
+#define offsetof(TYPE, FIELD) ((size_t) &((TYPE *)0)->FIELD)
+#endif
+
+#ifndef container_of
+#define container_of(PTR, TYPE, FIELD) ({			\
+	__typeof__(((TYPE *)0)->FIELD) *__FIELD_PTR = (PTR);	\
+	(TYPE *)((char *) __FIELD_PTR - offsetof(TYPE, FIELD));	\
+})
+#endif
 
 /* Buffer used to store int-to-ASCII conversions. Will only be implemented if
  * any of the related functions is implemented. The area is large enough to
@@ -60,6 +76,18 @@ int atoi(const char *s)
 	return atol(s);
 }
 
+static __attribute__((unused))
+void free(void *ptr)
+{
+	struct nolibc_heap *heap;
+
+	if (!ptr)
+		return;
+
+	heap = container_of(ptr, struct nolibc_heap, user_p);
+	munmap(heap, heap->len);
+}
+
 /* Converts the unsigned long integer <in> to its hex representation into
  * buffer <buffer>, which must be long enough to store the number and the
  * trailing zero (17 bytes for "ffffffffffffffff" or 9 for "ffffffff"). The
@@ -182,6 +210,57 @@ char *ltoa(long in)
 	return itoa_buffer;
 }
 
+static inline __attribute__((unused))
+void *malloc(size_t len)
+{
+	struct nolibc_heap *heap;
+
+	heap = mmap(NULL, sizeof(*heap) + len, PROT_READ|PROT_WRITE,
+		    MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
+	if (__builtin_expect(heap == MAP_FAILED, 0))
+		return NULL;
+
+	heap->len = sizeof(*heap) + len;
+	return heap->user_p;
+}
+
+static inline __attribute__((unused))
+void *calloc(size_t size, size_t nmemb)
+{
+	void *orig;
+	size_t res = 0;
+
+	if (__builtin_expect(__builtin_mul_overflow(nmemb, size, &res), 0)) {
+		SET_ERRNO(ENOMEM);
+		return NULL;
+	}
+
+	/*
+	 * No need to zero the heap, the MAP_ANONYMOUS in malloc()
+	 * already does it.
+	 */
+	return malloc(res);
+}
+
+static inline __attribute__((unused))
+void *realloc(void *old_ptr, size_t new_size)
+{
+	struct nolibc_heap *heap;
+	void *ret;
+
+	if (!old_ptr)
+		return malloc(new_size);
+
+	ret = malloc(new_size);
+	if (__builtin_expect(!ret, 0))
+		return NULL;
+
+	heap = container_of(old_ptr, struct nolibc_heap, user_p);
+	memcpy(ret, heap->user_p, heap->len);
+	munmap(heap, heap->len);
+	return ret;
+}
+
 /* converts unsigned long integer <in> to a string using the static itoa_buffer
  * and returns the pointer to that string.
  */
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
                   ` (4 preceding siblings ...)
  2022-03-20  9:37 ` [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
@ 2022-03-20  9:37 ` Ammar Faizi
  2022-03-20 15:55   ` Alviro Iskandar Setiawan
  2022-03-21  7:53   ` Willy Tarreau
  5 siblings, 2 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20  9:37 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, Ammar Faizi

Add strdup and strndup support. These functions are only available on
architectures that have my_syscall6() macro from nolibc.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 tools/include/nolibc/string.h | 68 +++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
index 4554b6fcb400..413c65f7c853 100644
--- a/tools/include/nolibc/string.h
+++ b/tools/include/nolibc/string.h
@@ -9,6 +9,10 @@
 
 #include "std.h"
 
+static void free(void *ptr);
+static void *malloc(size_t len);
+static void *realloc(void *old_ptr, size_t new_size);
+
 /*
  * As much as possible, please keep functions alphabetically sorted.
  */
@@ -127,6 +131,70 @@ size_t nolibc_strlen(const char *str)
 		nolibc_strlen((str));           \
 })
 
+static __attribute__((unused))
+char *strdup(const char *str)
+{
+	size_t allocated = 2048;
+	size_t i;
+	char *ret;
+	char *tmp;
+
+	ret = malloc(allocated);
+	if (__builtin_expect(!ret, 0))
+		return NULL;
+
+	i = 0;
+	for (;;) {
+		char c = *str;
+		if (!c)
+			break;
+
+		if (i == allocated) {
+			allocated += 2048;
+			tmp = realloc(ret, allocated);
+			if (__builtin_expect(!tmp, 0)) {
+				free(ret);
+				return NULL;
+			}
+			ret = tmp;
+		}
+
+		ret[i++] = c;
+		str++;
+	}
+
+	ret[i] = '\0';
+	return ret;
+}
+
+static __attribute__((unused))
+char *strndup(const char *str, size_t maxlen)
+{
+	size_t i;
+	char *ret;
+
+	ret = malloc(maxlen + 1);
+	if (__builtin_expect(!ret, 0))
+		return NULL;
+
+	i = 0;
+	for (;;) {
+		char c = *str;
+		if (!c)
+			break;
+
+		if (i == maxlen)
+			break;
+
+		ret[i++] = c;
+		str++;
+	}
+
+	ret[i] = '\0';
+	return ret;
+}
+
+
 static __attribute__((unused))
 size_t strlcat(char *dst, const char *src, size_t size)
 {
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20  9:37 ` [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
@ 2022-03-20 10:33   ` Alviro Iskandar Setiawan
  2022-03-20 10:42     ` Alviro Iskandar Setiawan
  2022-03-20 13:10   ` David Laight
  1 sibling, 1 reply; 29+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-20 10:33 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

On Sun, Mar 20, 2022 at 4:37 PM Ammar Faizi wrote:
> In i386, the 6th argument of syscall goes in %ebp. However, both Clang
> and GCC cannot use %ebp in the clobber list and in the "r" constraint
> without using -fomit-frame-pointer. To make it always available for any
> kind of compilation, the below workaround is implemented.
>
> For clang (the Assembly statement can't clobber %ebp):
>   1) Save the %ebp value to the redzone area -4(%esp).
>   2) Load the 6-th argument from memory to %ebp.
>   3) Subtract the %esp by 4.
>   4) Do the syscall (int $0x80).
>   5) Pop %ebp.

I don't think you can safely use redzone from inline Assembly. The
compiler may also use redzone for a leaf function. In case the syscall
is done at the same time, your %ebp saving will clobber the redzone
that the compiler uses.

> For GCC, fortunately it has a #pragma that can force a specific function
> to be compiled with -fomit-frame-pointer, so it can always use "r"(var)
> where `var` is a variable bound to %ebp.
>
> Cc: x86@kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
[...]
> +#if defined(__clang__)
> +static inline long ____do_syscall6(long eax, long ebx, long ecx, long edx,
> +                                  long esi, long edi, long ebp)
> +{
> +       __asm__ volatile (
> +               "movl   %%ebp, -4(%%esp)\n\t"
> +               "movl   %[arg6], %%ebp\n\t"
> +               "subl   $4, %%esp\n\t"
> +               "int    $0x80\n\t"
> +               "popl   %%ebp\n\t"
> +               : "=a"(eax)
> +               : "a"(eax), "b"(ebx), "c"(ecx), "d"(edx), "S"(esi), "D"(edi),
> +                 [arg6]"m"(ebp)
> +               : "memory", "cc"
> +       );
> +       return eax;
> +}
> +

-4(%esp) may be used by the compiler on a leaf call, you can't clobber that.

-- Viro

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20 10:33   ` Alviro Iskandar Setiawan
@ 2022-03-20 10:42     ` Alviro Iskandar Setiawan
  2022-03-20 15:09       ` Ammar Faizi
  0 siblings, 1 reply; 29+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-20 10:42 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

On Sun, Mar 20, 2022 at 5:33 PM Alviro Iskandar Setiawan wrote:
> On Sun, Mar 20, 2022 at 4:37 PM Ammar Faizi wrote:
> > In i386, the 6th argument of syscall goes in %ebp. However, both Clang
> > and GCC cannot use %ebp in the clobber list and in the "r" constraint
> > without using -fomit-frame-pointer. To make it always available for any
> > kind of compilation, the below workaround is implemented.
> >
> > For clang (the Assembly statement can't clobber %ebp):
> >   1) Save the %ebp value to the redzone area -4(%esp).
> >   2) Load the 6-th argument from memory to %ebp.
> >   3) Subtract the %esp by 4.
> >   4) Do the syscall (int $0x80).
> >   5) Pop %ebp.
>
> I don't think you can safely use redzone from inline Assembly. The
> compiler may also use redzone for a leaf function. In case the syscall
> is done at the same time, your %ebp saving will clobber the redzone
> that the compiler uses.
>
> > For GCC, fortunately it has a #pragma that can force a specific function
> > to be compiled with -fomit-frame-pointer, so it can always use "r"(var)
> > where `var` is a variable bound to %ebp.
> >
> > Cc: x86@kernel.org
> > Cc: llvm@lists.linux.dev
> > Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> [...]
> > +#if defined(__clang__)
> > +static inline long ____do_syscall6(long eax, long ebx, long ecx, long edx,
> > +                                  long esi, long edi, long ebp)
> > +{
> > +       __asm__ volatile (
> > +               "movl   %%ebp, -4(%%esp)\n\t"
> > +               "movl   %[arg6], %%ebp\n\t"
> > +               "subl   $4, %%esp\n\t"
> > +               "int    $0x80\n\t"
> > +               "popl   %%ebp\n\t"
> > +               : "=a"(eax)
> > +               : "a"(eax), "b"(ebx), "c"(ecx), "d"(edx), "S"(esi), "D"(edi),
> > +                 [arg6]"m"(ebp)
> > +               : "memory", "cc"
> > +       );
> > +       return eax;
> > +}
> > +
>
> -4(%esp) may be used by the compiler on a leaf call, you can't clobber that.

Using xchgl to preserve %ebp in the same place where the arg6 is
stored in memory is a better solution and doesn't clobber anything.

    xchgl %ebp, %[arg6]
    int    $0x80
    xchgl %ebp, %[arg6]

-- Viro

^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20  9:37 ` [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
  2022-03-20 10:33   ` Alviro Iskandar Setiawan
@ 2022-03-20 13:10   ` David Laight
  2022-03-20 14:01     ` Willy Tarreau
  2022-03-20 15:04     ` Ammar Faizi
  1 sibling, 2 replies; 29+ messages in thread
From: David Laight @ 2022-03-20 13:10 UTC (permalink / raw)
  To: 'Ammar Faizi', Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

From: Ammar Faizi
> Sent: 20 March 2022 09:38
> 
> In i386, the 6th argument of syscall goes in %ebp. However, both Clang
> and GCC cannot use %ebp in the clobber list and in the "r" constraint
> without using -fomit-frame-pointer. To make it always available for any
> kind of compilation, the below workaround is implemented.
> 
> For clang (the Assembly statement can't clobber %ebp):
>   1) Save the %ebp value to the redzone area -4(%esp).

i386 doesn't have a redzone.
If you get a signal it will trash -4(%sp)

>   2) Load the 6-th argument from memory to %ebp.
>   3) Subtract the %esp by 4.
>   4) Do the syscall (int $0x80).
>   5) Pop %ebp.
> 
> For GCC, fortunately it has a #pragma that can force a specific function
> to be compiled with -fomit-frame-pointer, so it can always use "r"(var)
> where `var` is a variable bound to %ebp.

How is that going to work for an inlined functon?

And using xchg is slow - it is always locked.

One possibility might be to do:
	push arg6
	push %ebp
	mov  %ebp, 4(%sp)
	int  0x80
	pop  %ebp
	add  %esp,4

Although I'm not sure you really want to allocate 4k pages
for every malloc() call.

Probably better to write a mini 'libc' that uses sbrk()
and a best fit scan of a linear free list.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20 13:10   ` David Laight
@ 2022-03-20 14:01     ` Willy Tarreau
  2022-03-20 15:04     ` Ammar Faizi
  1 sibling, 0 replies; 29+ messages in thread
From: Willy Tarreau @ 2022-03-20 14:01 UTC (permalink / raw)
  To: David Laight
  Cc: 'Ammar Faizi',
	Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

Hi David,

On Sun, Mar 20, 2022 at 01:10:33PM +0000, David Laight wrote:
> And using xchg is slow - it is always locked.

Note that we don't really care here, as it remains minimal compared to
an mmap() call.

> One possibility might be to do:
> 	push arg6
> 	push %ebp
> 	mov  %ebp, 4(%sp)
> 	int  0x80
> 	pop  %ebp
> 	add  %esp,4
> 
> Although I'm not sure you really want to allocate 4k pages
> for every malloc() call.

Well, it depends. I would argue that we don't even need malloc() but
on the other hand this is essentially used to write small regtests so
we don't really care about the waste here if someone really needs it.

I'd rather get Ammar's motivations for malloc() in the first place.

Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20 13:10   ` David Laight
  2022-03-20 14:01     ` Willy Tarreau
@ 2022-03-20 15:04     ` Ammar Faizi
  2022-03-20 18:22       ` David Laight
  1 sibling, 1 reply; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20 15:04 UTC (permalink / raw)
  To: David Laight, Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

On 3/20/22 8:10 PM, David Laight wrote:
> From: Ammar Faizi
>> Sent: 20 March 2022 09:38
>>
>> In i386, the 6th argument of syscall goes in %ebp. However, both Clang
>> and GCC cannot use %ebp in the clobber list and in the "r" constraint
>> without using -fomit-frame-pointer. To make it always available for any
>> kind of compilation, the below workaround is implemented.
>>
>> For clang (the Assembly statement can't clobber %ebp):
>>    1) Save the %ebp value to the redzone area -4(%esp).
> 
> i386 doesn't have a redzone.
> If you get a signal it will trash -4(%sp)

OK, I missed that one. Thanks for reviewing this.

>> For GCC, fortunately it has a #pragma that can force a specific function
>> to be compiled with -fomit-frame-pointer, so it can always use "r"(var)
>> where `var` is a variable bound to %ebp.
> 
> How is that going to work for an inlined functon?

It works, but obviously the inline modifier here is just a hint for the
compiler, not a requirement. I just took a look at the GCC generated code.
It doesn't inline the ____do_syscall6() despite it's marked as inline.

I think this one shouldn't be marked as inline. I will remove the inline
in the next version.

> And using xchg is slow - it is always locked.
> 
> One possibility might be to do:
> 	push arg6
> 	push %ebp
> 	mov  %ebp, 4(%sp)

Did you mean `mov 4(%esp), %ebp`?

> 	int  0x80
> 	pop  %ebp
> 	add  %esp,4

I think your solution is better than the xchg approach (with the 3rd line
fixed). Will take this in for the next version.

> Although I'm not sure you really want to allocate 4k pages
> for every malloc() call.
> 
> Probably better to write a mini 'libc' that uses sbrk()
> and a best fit scan of a linear free list.

^ This part is addressed by Willy's response. I will still go with mmap()
in the next version.

And yes, we do waste space for small allocations here, but we don't hit
the scale that the waste space will cause problem.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20 10:42     ` Alviro Iskandar Setiawan
@ 2022-03-20 15:09       ` Ammar Faizi
  0 siblings, 0 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20 15:09 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm,
	David Laight

On 3/20/22 5:42 PM, Alviro Iskandar Setiawan wrote:
> On Sun, Mar 20, 2022 at 5:33 PM Alviro Iskandar Setiawan wrote:
[...]
>> I don't think you can safely use redzone from inline Assembly. The
>> compiler may also use redzone for a leaf function. In case the syscall
>> is done at the same time, your %ebp saving will clobber the redzone
>> that the compiler uses.

It turned out we don't have a redzone for 32-bit.

>>
>> -4(%esp) may be used by the compiler on a leaf call, you can't clobber that.

Yeah, this is still wrong even with a redzone.

> Using xchgl to preserve %ebp in the same place where the arg6 is
> stored in memory is a better solution and doesn't clobber anything.
> 
>      xchgl %ebp, %[arg6]
>      int    $0x80
>      xchgl %ebp, %[arg6]

Addressed your review in my response to David.

Thanks!

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20  9:37 ` [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
@ 2022-03-20 15:50   ` Alviro Iskandar Setiawan
  2022-03-20 16:10     ` Ammar Faizi
  2022-03-20 16:16   ` Willy Tarreau
  1 sibling, 1 reply; 29+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-20 15:50 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Sun, Mar 20, 2022 at 4:37 PM Ammar Faizi wrote:
> +void *realloc(void *old_ptr, size_t new_size)
> +{
> +       struct nolibc_heap *heap;
> +       void *ret;
> +
> +       if (!old_ptr)
> +               return malloc(new_size);
> +
> +       ret = malloc(new_size);
> +       if (__builtin_expect(!ret, 0))
> +               return NULL;
> +
> +       heap = container_of(old_ptr, struct nolibc_heap, user_p);
> +       memcpy(ret, heap->user_p, heap->len);
> +       munmap(heap, heap->len);
> +       return ret;
> +}

This better be simplified like this, so only have 1 malloc() call that
applies to both branches.

  void *realloc(void *old_ptr, size_t new_size)
  {
         struct nolibc_heap *heap;
         void *ret;

         ret = malloc(new_size);
         if (__builtin_expect(!ret, 0))
                 return NULL;

         if (!old_ptr)
                 return ret;

         heap = container_of(old_ptr, struct nolibc_heap, user_p);
         memcpy(ret, heap->user_p, heap->len);
         munmap(heap, heap->len);
         return ret;
  }

-- Viro

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-20  9:37 ` [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi
@ 2022-03-20 15:55   ` Alviro Iskandar Setiawan
  2022-03-20 16:10     ` Ammar Faizi
  2022-03-21  7:53   ` Willy Tarreau
  1 sibling, 1 reply; 29+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-20 15:55 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Sun, Mar 20, 2022 at 4:37 PM Ammar Faizi wrote:
> +}
> +
> +

(Trivial) Got double newlines here, one newline should be good.

>  static __attribute__((unused))
>  size_t strlcat(char *dst, const char *src, size_t size)
>  {

-- Viro

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20 15:50   ` Alviro Iskandar Setiawan
@ 2022-03-20 16:10     ` Ammar Faizi
  0 siblings, 0 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20 16:10 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On 3/20/22 10:50 PM, Alviro Iskandar Setiawan wrote:
> This better be simplified like this, so only have 1 malloc() call that
> applies to both branches.
> 
>    void *realloc(void *old_ptr, size_t new_size)
>    {
>           struct nolibc_heap *heap;
>           void *ret;
> 
>           ret = malloc(new_size);
>           if (__builtin_expect(!ret, 0))
>                   return NULL;
> 
>           if (!old_ptr)
>                   return ret;
> 
>           heap = container_of(old_ptr, struct nolibc_heap, user_p);
>           memcpy(ret, heap->user_p, heap->len);
>           munmap(heap, heap->len);
>           return ret;
>    }

That looks better, will take this for the v2. Thanks!

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-20 15:55   ` Alviro Iskandar Setiawan
@ 2022-03-20 16:10     ` Ammar Faizi
  0 siblings, 0 replies; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20 16:10 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Willy Tarreau, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On 3/20/22 10:55 PM, Alviro Iskandar Setiawan wrote:
> On Sun, Mar 20, 2022 at 4:37 PM Ammar Faizi wrote:
>> +}
>> +
>> +
> 
> (Trivial) Got double newlines here, one newline should be good.

OK, fixed.

>>   static __attribute__((unused))
>>   size_t strlcat(char *dst, const char *src, size_t size)
>>   {

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20  9:37 ` [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
  2022-03-20 15:50   ` Alviro Iskandar Setiawan
@ 2022-03-20 16:16   ` Willy Tarreau
  2022-03-20 16:36     ` Ammar Faizi
  1 sibling, 1 reply; 29+ messages in thread
From: Willy Tarreau @ 2022-03-20 16:16 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

Ammar,

a few points below:

On Sun, Mar 20, 2022 at 04:37:49PM +0700, Ammar Faizi wrote:
> +struct nolibc_heap {
> +	size_t	len;
> +	char	user_p[] __attribute__((__aligned__));
> +};

Note that many programs assume that malloc() returns a field aligned
to 2*sizeof(pointer) and unless I'm mistaken, above the user pointer
will only be aligned by one pointer. This may have an impact when the
compiler decides to use SIMD instructions.

> +#ifndef offsetof
> +#define offsetof(TYPE, FIELD) ((size_t) &((TYPE *)0)->FIELD)
> +#endif
> +
> +#ifndef container_of
> +#define container_of(PTR, TYPE, FIELD) ({			\
> +	__typeof__(((TYPE *)0)->FIELD) *__FIELD_PTR = (PTR);	\
> +	(TYPE *)((char *) __FIELD_PTR - offsetof(TYPE, FIELD));	\
> +})
> +#endif

These ones are independent on the malloc code and should move to a
different patch and likely to a different file. I'm seeing we already
have a few macros in types.h and since it's shared by almost everything
it might be more suitable there.

Thanks!
Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20 16:16   ` Willy Tarreau
@ 2022-03-20 16:36     ` Ammar Faizi
  2022-03-20 16:46       ` Willy Tarreau
  0 siblings, 1 reply; 29+ messages in thread
From: Ammar Faizi @ 2022-03-20 16:36 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On 3/20/22 11:16 PM, Willy Tarreau wrote:
> Ammar,
> 
> a few points below:
> 
> On Sun, Mar 20, 2022 at 04:37:49PM +0700, Ammar Faizi wrote:
>> +struct nolibc_heap {
>> +	size_t	len;
>> +	char	user_p[] __attribute__((__aligned__));
>> +};
> 
> Note that many programs assume that malloc() returns a field aligned
> to 2*sizeof(pointer) and unless I'm mistaken, above the user pointer
> will only be aligned by one pointer. This may have an impact when the
> compiler decides to use SIMD instructions.

Section 7.20.3 of C99 states this about `malloc()`:
"""
   The pointer returned if the allocation succeeds is suitably aligned
   so that it may be assigned to a pointer to any type of object.
"""

And this is what GCC doc says about __attribute__((__aligned__)):
"""
   The aligned attribute specifies a minimum alignment for the variable
   or structure field, measured in bytes. When specified, alignment must
   be an integer constant power of 2. Specifying no alignment argument
   implies the maximum alignment for the target, which is often, but by
   no means always, 8 or 16 bytes.
"""

Link: https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes

Simple experiment on Linux x86-64...

```
ammarfaizi2@integral2:/tmp$ cat > test.c
#include <stdio.h>
int main(void)
{
	printf("alignof = %zu\n", __alignof__(long double));
	return 0;
}
ammarfaizi2@integral2:/tmp$ gcc -o test test.c
ammarfaizi2@integral2:/tmp$ ./test
alignof = 16
ammarfaizi2@integral2:/tmp$
```

We have `long double` which requires 16 byte alignment. So
__attribute__((__aligned__)) should cover this. And yes, it's true that
it's 2*sizeof(void*), but importantly for the above reason.

>> +#ifndef offsetof
>> +#define offsetof(TYPE, FIELD) ((size_t) &((TYPE *)0)->FIELD)
>> +#endif
>> +
>> +#ifndef container_of
>> +#define container_of(PTR, TYPE, FIELD) ({			\
>> +	__typeof__(((TYPE *)0)->FIELD) *__FIELD_PTR = (PTR);	\
>> +	(TYPE *)((char *) __FIELD_PTR - offsetof(TYPE, FIELD));	\
>> +})
>> +#endif
> 
> These ones are independent on the malloc code and should move to a
> different patch and likely to a different file. I'm seeing we already
> have a few macros in types.h and since it's shared by almost everything
> it might be more suitable there.

OK, will do it in the v2. Thanks!

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  2022-03-20 16:36     ` Ammar Faizi
@ 2022-03-20 16:46       ` Willy Tarreau
  0 siblings, 0 replies; 29+ messages in thread
From: Willy Tarreau @ 2022-03-20 16:46 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Sun, Mar 20, 2022 at 11:36:55PM +0700, Ammar Faizi wrote:
> And this is what GCC doc says about __attribute__((__aligned__)):
> """
>   The aligned attribute specifies a minimum alignment for the variable
>   or structure field, measured in bytes. When specified, alignment must
>   be an integer constant power of 2. Specifying no alignment argument
>   implies the maximum alignment for the target, which is often, but by
>   no means always, 8 or 16 bytes.
> """
> 
> Link: https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes

OK then that's fine, thank you. I thought it would force the alignment
to the type itself.

> Simple experiment on Linux x86-64...

That's even easier checked like this:

  $ cat > c.c <<EOF
  struct blah {
          char a;
          char b __attribute__((__aligned__));
  } blah;
  EOF
  $ gcc -g -c c.c
  $ pahole c.o
  struct blah {
          char                       a;                    /*     0     1 */
  
          /* XXX 15 bytes hole, try to pack */
  
          char                       b __attribute__((__aligned__(16))); /*    16     1 */
  
          /* size: 32, cachelines: 1, members: 2 */
          /* sum members: 2, holes: 1, sum holes: 15 */
          /* padding: 15 */
          /* forced alignments: 1, forced holes: 1, sum forced holes: 15 */
          /* last cacheline: 32 bytes */
  } __attribute__((__aligned__(16)));

Thank you!
Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments
  2022-03-20 15:04     ` Ammar Faizi
@ 2022-03-20 18:22       ` David Laight
  0 siblings, 0 replies; 29+ messages in thread
From: David Laight @ 2022-03-20 18:22 UTC (permalink / raw)
  To: 'Ammar Faizi', Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, x86, llvm

From: Ammar Faizi
> Sent: 20 March 2022 15:04
> On 3/20/22 8:10 PM, David Laight wrote:
> > From: Ammar Faizi
> >> Sent: 20 March 2022 09:38
> >>
> >> In i386, the 6th argument of syscall goes in %ebp. However, both Clang
> >> and GCC cannot use %ebp in the clobber list and in the "r" constraint
> >> without using -fomit-frame-pointer. To make it always available for any
> >> kind of compilation, the below workaround is implemented.
> >>
> >> For clang (the Assembly statement can't clobber %ebp):
> >>    1) Save the %ebp value to the redzone area -4(%esp).
> >
> > i386 doesn't have a redzone.
> > If you get a signal it will trash -4(%sp)
> 
> OK, I missed that one. Thanks for reviewing this.
> 
...
> >
> > One possibility might be to do:
> > 	push arg6
> > 	push %ebp
> > 	mov  %ebp, 4(%sp)
> 
> Did you mean `mov 4(%esp), %ebp`?
> 
> > 	int  0x80
> > 	pop  %ebp
> > 	add  %esp,4
> 
> I think your solution is better than the xchg approach (with the 3rd line
> fixed). Will take this in for the next version.

It has to be said that although I've been writing x86 asm
for 40 years (and others for longer) I can never actually
remember the exact syntax or order of the operands!
Probably because it is randomly different between assemblers.
You want the 'memory read' instruction: 8b /r.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang
  2022-03-20  9:37 ` [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang Ammar Faizi
@ 2022-03-20 19:16   ` Willy Tarreau
  2022-03-21 11:38     ` Ammar Faizi
  0 siblings, 1 reply; 29+ messages in thread
From: Willy Tarreau @ 2022-03-20 19:16 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, llvm

Hi Ammar,

I've had a look at this one.

On Sun, Mar 20, 2022 at 04:37:46PM +0700, Ammar Faizi wrote:
> Budilig with clang yields the following error:
  ^^^^^^^
BTW please fix the typo in the final commit message.

> @@ -183,7 +183,9 @@ struct sys_stat_struct {
>  
>  /* startup code */
>  asm(".section .text\n"
> +#if !defined(__clang__)
>      ".weak _start\n"
> +#endif
>      ".global _start\n"
>      "_start:\n"

So it seems that I was wrong and that .weak is an alternate for .global
and not a complement. As such, instead of adding all these #if, please
simply remove all .global.

Thanks!
Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-20  9:37 ` [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi
  2022-03-20 15:55   ` Alviro Iskandar Setiawan
@ 2022-03-21  7:53   ` Willy Tarreau
  2022-03-21  8:16     ` Alviro Iskandar Setiawan
  2022-03-21 11:36     ` Ammar Faizi
  1 sibling, 2 replies; 29+ messages in thread
From: Willy Tarreau @ 2022-03-21  7:53 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

Hi Ammar,

On Sun, Mar 20, 2022 at 04:37:50PM +0700, Ammar Faizi wrote:
> Add strdup and strndup support. These functions are only available on
> architectures that have my_syscall6() macro from nolibc.
> 
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> ---
>  tools/include/nolibc/string.h | 68 +++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
> index 4554b6fcb400..413c65f7c853 100644
> --- a/tools/include/nolibc/string.h
> +++ b/tools/include/nolibc/string.h
> @@ -9,6 +9,10 @@
>  
>  #include "std.h"
>  
> +static void free(void *ptr);
> +static void *malloc(size_t len);
> +static void *realloc(void *old_ptr, size_t new_size);

Better include the required h files here.

>  /*
>   * As much as possible, please keep functions alphabetically sorted.
>   */
> @@ -127,6 +131,70 @@ size_t nolibc_strlen(const char *str)
>  		nolibc_strlen((str));           \
>  })
>  
> +static __attribute__((unused))
> +char *strdup(const char *str)
> +{
> +	size_t allocated = 2048;
> +	size_t i;
> +	char *ret;
> +	char *tmp;
> +
> +	ret = malloc(allocated);
> +	if (__builtin_expect(!ret, 0))
> +		return NULL;
> +
> +	i = 0;
> +	for (;;) {
> +		char c = *str;
> +		if (!c)
> +			break;
> +
> +		if (i == allocated) {
> +			allocated += 2048;
> +			tmp = realloc(ret, allocated);
> +			if (__builtin_expect(!tmp, 0)) {
> +				free(ret);
> +				return NULL;
> +			}
> +			ret = tmp;
> +		}
> +
> +		ret[i++] = c;
> +		str++;
> +	}
> +
> +	ret[i] = '\0';
> +	return ret;
> +}

This version is suboptimal in terms of code size, CPU usage and memory
usage. And it even seems it contains a buffer overflow: if the string
is exactly a multiple of 2048, it seems to me that you'll write the
trailing zero past the end. Please instead use the more intuitive form
below (not tested but you get the idea):

	size_t len = strlen(str);
	char *ret = malloc(len + 1);
	if (ret)
		memcpy(ret, str, len);
	return ret;

> +static __attribute__((unused))
> +char *strndup(const char *str, size_t maxlen)
> +{
> +	size_t i;
> +	char *ret;
> +
> +	ret = malloc(maxlen + 1);
> +	if (__builtin_expect(!ret, 0))
> +		return NULL;
> +
> +	i = 0;
> +	for (;;) {
> +		char c = *str;
> +		if (!c)
> +			break;
> +
> +		if (i == maxlen)
> +			break;
> +
> +		ret[i++] = c;
> +		str++;
> +	}
> +
> +	ret[i] = '\0';
> +	return ret;
> +}

Here it can cost quite a lot for large values of maxlen. Please just use
a variant of the proposal above like this one:

	size_t len;
	char *ret;

	len = strlen(str);
	if (len > maxlen)
		len = maxlen;
	ret = malloc(len + 1);
	if (ret)
		memcpy(ret, str, len);
	return ret;

Thanks,
Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-21  7:53   ` Willy Tarreau
@ 2022-03-21  8:16     ` Alviro Iskandar Setiawan
  2022-03-21  8:51       ` Willy Tarreau
  2022-03-21 11:36     ` Ammar Faizi
  1 sibling, 1 reply; 29+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-21  8:16 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Ammar Faizi, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Mon, Mar 21, 2022 at 2:53 PM Willy Tarreau wrote:
> Here it can cost quite a lot for large values of maxlen. Please just use
> a variant of the proposal above like this one:
>
>         size_t len;
>         char *ret;
>
>         len = strlen(str);
>         if (len > maxlen)
>                 len = maxlen;
>         ret = malloc(len + 1);
>         if (ret)
>                 memcpy(ret, str, len);
>         return ret;

Maybe better to use strnlen(), see the detail at man 3 strnlen.

  size_t strnlen(const char *s, size_t maxlen);

The strnlen() function returns the number of bytes in the string
pointed to by s, excluding the terminating null byte ('\0'), but at
most maxlen. In doing this, strnlen() looks only at the first maxlen
characters in the string pointed to by s and never beyond s[maxlen-1].

Should be trivial to add strnlen() with a separate patch before this patch.

So it can be:

    size_t len;
    char *ret;

    len = strnlen(str, maxlen);
    ret = malloc(len + 1);
    if (__builtin_expect(ret != NULL, 1)) {
        memcpy(ret, str, len);
        ret[len] = '\0';
    }
    return ret;

Thoughts?

-- Viro

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-21  8:16     ` Alviro Iskandar Setiawan
@ 2022-03-21  8:51       ` Willy Tarreau
  0 siblings, 0 replies; 29+ messages in thread
From: Willy Tarreau @ 2022-03-21  8:51 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Ammar Faizi, Paul E. McKenney, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Mon, Mar 21, 2022 at 03:16:54PM +0700, Alviro Iskandar Setiawan wrote:
> On Mon, Mar 21, 2022 at 2:53 PM Willy Tarreau wrote:
> > Here it can cost quite a lot for large values of maxlen. Please just use
> > a variant of the proposal above like this one:
> >
> >         size_t len;
> >         char *ret;
> >
> >         len = strlen(str);
> >         if (len > maxlen)
> >                 len = maxlen;
> >         ret = malloc(len + 1);
> >         if (ret)
> >                 memcpy(ret, str, len);
> >         return ret;
> 
> Maybe better to use strnlen(), see the detail at man 3 strnlen.
> 
>   size_t strnlen(const char *s, size_t maxlen);
> 
> The strnlen() function returns the number of bytes in the string
> pointed to by s, excluding the terminating null byte ('\0'), but at
> most maxlen. In doing this, strnlen() looks only at the first maxlen
> characters in the string pointed to by s and never beyond s[maxlen-1].
> 
> Should be trivial to add strnlen() with a separate patch before this patch.
> 
> So it can be:
> 
>     size_t len;
>     char *ret;
> 
>     len = strnlen(str, maxlen);
>     ret = malloc(len + 1);
>     if (__builtin_expect(ret != NULL, 1)) {
>         memcpy(ret, str, len);
>         ret[len] = '\0';
>     }
>     return ret;
> 
> Thoughts?

I thought about it as well and while I was seeking the simplest route,
I agree it would indeed be cleaner.

Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-21  7:53   ` Willy Tarreau
  2022-03-21  8:16     ` Alviro Iskandar Setiawan
@ 2022-03-21 11:36     ` Ammar Faizi
  2022-03-21 11:43       ` Willy Tarreau
  1 sibling, 1 reply; 29+ messages in thread
From: Ammar Faizi @ 2022-03-21 11:36 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On 3/21/22 2:53 PM, Willy Tarreau wrote:
> Hi Ammar,
[...]
>> +static void free(void *ptr);
>> +static void *malloc(size_t len);
>> +static void *realloc(void *old_ptr, size_t new_size);
> 
> Better include the required h files here.

I can't do that, in nolibc.h, we have something like this:

```
   #include "stdlib.h" <--- We inlcude string.h from here
   #include "string.h" <--- This is a no-op.
```

Note, stdlib.h is included first before string.h, next, in stdlib.h, we
have this:

```
   #include "string.h"

   // malloc, calloc, free here
```

If I include "stdlib.h" in "string.h", it will just be a no-op, and the
declarations will not be taken, because the declarations happen after
#include "string.h". So it doesn't work. It's somewhat circular dependency.

   stdlib.h needs string.h
   string.h needs stdlib.h

One of them must fully see the other before they can use the defined functions
in another header.

Suggestion welcome...

I am thinking of creating a new header just for the forward declarations
where all function declarations live there, we just split off the real
functions' body.

[...]
> 
> This version is suboptimal in terms of code size, CPU usage and memory
> usage. And it even seems it contains a buffer overflow: if the string
> is exactly a multiple of 2048, it seems to me that you'll write the
> trailing zero past the end. Please instead use the more intuitive form
> below (not tested but you get the idea):
> 
> 	size_t len = strlen(str);
> 	char *ret = malloc(len + 1);
> 	if (ret)
> 		memcpy(ret, str, len);
> 	return ret;

Ah right, that's indeed overflow. Will fold this in as you suggested.

[...]
> Here it can cost quite a lot for large values of maxlen. Please just use
> a variant of the proposal above like this one:
> 
> 	size_t len;
> 	char *ret;
> 
> 	len = strlen(str);
> 	if (len > maxlen)
> 		len = maxlen;
> 	ret = malloc(len + 1);
> 	if (ret)
> 		memcpy(ret, str, len);
> 	return ret;

Will take the Alviro's suggestion for this part...

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang
  2022-03-20 19:16   ` Willy Tarreau
@ 2022-03-21 11:38     ` Ammar Faizi
  2022-03-21 17:27       ` Nick Desaulniers
  0 siblings, 1 reply; 29+ messages in thread
From: Ammar Faizi @ 2022-03-21 11:38 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List, llvm

On 3/21/22 2:16 AM, Willy Tarreau wrote:
> Hi Ammar,
> 
> I've had a look at this one.
> 
> On Sun, Mar 20, 2022 at 04:37:46PM +0700, Ammar Faizi wrote:
>> Budilig with clang yields the following error:
>    ^^^^^^^
> BTW please fix the typo in the final commit message.
> 
>> @@ -183,7 +183,9 @@ struct sys_stat_struct {
>>   
>>   /* startup code */
>>   asm(".section .text\n"
>> +#if !defined(__clang__)
>>       ".weak _start\n"
>> +#endif
>>       ".global _start\n"
>>       "_start:\n"
> 
> So it seems that I was wrong and that .weak is an alternate for .global
> and not a complement. As such, instead of adding all these #if, please
> simply remove all .global.

Will fix this in the next version.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()`
  2022-03-21 11:36     ` Ammar Faizi
@ 2022-03-21 11:43       ` Willy Tarreau
  0 siblings, 0 replies; 29+ messages in thread
From: Willy Tarreau @ 2022-03-21 11:43 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Paul E. McKenney, Alviro Iskandar Setiawan, Nugraha,
	Linux Kernel Mailing List, GNU/Weeb Mailing List

On Mon, Mar 21, 2022 at 06:36:37PM +0700, Ammar Faizi wrote:
> On 3/21/22 2:53 PM, Willy Tarreau wrote:
> > Hi Ammar,
> [...]
> > > +static void free(void *ptr);
> > > +static void *malloc(size_t len);
> > > +static void *realloc(void *old_ptr, size_t new_size);
> > 
> > Better include the required h files here.
> 
> I can't do that, in nolibc.h, we have something like this:
> 
> ```
>   #include "stdlib.h" <--- We inlcude string.h from here
>   #include "string.h" <--- This is a no-op.
> ```
> 
> Note, stdlib.h is included first before string.h, next, in stdlib.h, we
> have this:
> 
> ```
>   #include "string.h"
> 
>   // malloc, calloc, free here
> ```
> 
> If I include "stdlib.h" in "string.h", it will just be a no-op, and the
> declarations will not be taken, because the declarations happen after
> #include "string.h". So it doesn't work. It's somewhat circular dependency.
> 
>   stdlib.h needs string.h
>   string.h needs stdlib.h
> 
> One of them must fully see the other before they can use the defined functions
> in another header.

OK, usual stuff indeed.

> Suggestion welcome...

Then just leave it as-is.

> I am thinking of creating a new header just for the forward declarations
> where all function declarations live there, we just split off the real
> functions' body.

That's why I'm doing in my projects for the exact reason above. But
here we only have static functions so this will increase the burden to
contribute. Better wait for the situation to reach a point where we're
certain there will be some benefit in doing that.

Thanks,
Willy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang
  2022-03-21 11:38     ` Ammar Faizi
@ 2022-03-21 17:27       ` Nick Desaulniers
  0 siblings, 0 replies; 29+ messages in thread
From: Nick Desaulniers @ 2022-03-21 17:27 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Willy Tarreau, Paul E. McKenney, Alviro Iskandar Setiawan,
	Nugraha, Linux Kernel Mailing List, GNU/Weeb Mailing List, llvm

On Mon, Mar 21, 2022 at 4:38 AM Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
>
> On 3/21/22 2:16 AM, Willy Tarreau wrote:
> > Hi Ammar,
> >
> > I've had a look at this one.
> >
> > On Sun, Mar 20, 2022 at 04:37:46PM +0700, Ammar Faizi wrote:
> >> Budilig with clang yields the following error:
> >    ^^^^^^^
> > BTW please fix the typo in the final commit message.
> >
> >> @@ -183,7 +183,9 @@ struct sys_stat_struct {
> >>
> >>   /* startup code */
> >>   asm(".section .text\n"
> >> +#if !defined(__clang__)
> >>       ".weak _start\n"
> >> +#endif
> >>       ".global _start\n"
> >>       "_start:\n"
> >
> > So it seems that I was wrong and that .weak is an alternate for .global
> > and not a complement. As such, instead of adding all these #if, please
> > simply remove all .global.

See also:
commit 4d6ffa27b8e5 ("x86/lib: Change .weak to SYM_FUNC_START_WEAK for
arch/x86/lib/mem*_64.S")
commit ec9d78070de9 ("arm64: Change .weak to SYM_FUNC_START_WEAK_PI
for arch/arm64/lib/mem*.S")

Also, please note in the commit message that this diagnostic comes
from using clang as the assembler (which is clang's default behavior
unless -fno-integrated-as is passed).

>
> Will fix this in the next version.
>
> --
> Ammar Faizi
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2022-03-21 17:27 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-20  9:37 [RFC PATCH v1 0/6] Add dynamic memory allocator support for nolibc Ammar Faizi
2022-03-20  9:37 ` [RFC PATCH v1 1/6] tools/nolibc: x86-64: Update System V ABI document link Ammar Faizi
2022-03-20  9:37 ` [RFC PATCH v1 2/6] tools/nolibc: Make the entry point not weak for clang Ammar Faizi
2022-03-20 19:16   ` Willy Tarreau
2022-03-21 11:38     ` Ammar Faizi
2022-03-21 17:27       ` Nick Desaulniers
2022-03-20  9:37 ` [RFC PATCH v1 3/6] tools/nolibc: i386: Implement syscall with 6 arguments Ammar Faizi
2022-03-20 10:33   ` Alviro Iskandar Setiawan
2022-03-20 10:42     ` Alviro Iskandar Setiawan
2022-03-20 15:09       ` Ammar Faizi
2022-03-20 13:10   ` David Laight
2022-03-20 14:01     ` Willy Tarreau
2022-03-20 15:04     ` Ammar Faizi
2022-03-20 18:22       ` David Laight
2022-03-20  9:37 ` [RFC PATCH v1 4/6] tools/nolibc/sys: Implement `mmap()` and `munmap()` Ammar Faizi
2022-03-20  9:37 ` [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()` Ammar Faizi
2022-03-20 15:50   ` Alviro Iskandar Setiawan
2022-03-20 16:10     ` Ammar Faizi
2022-03-20 16:16   ` Willy Tarreau
2022-03-20 16:36     ` Ammar Faizi
2022-03-20 16:46       ` Willy Tarreau
2022-03-20  9:37 ` [RFC PATCH v1 6/6] tools/include/string: Implement `strdup()` and `strndup()` Ammar Faizi
2022-03-20 15:55   ` Alviro Iskandar Setiawan
2022-03-20 16:10     ` Ammar Faizi
2022-03-21  7:53   ` Willy Tarreau
2022-03-21  8:16     ` Alviro Iskandar Setiawan
2022-03-21  8:51       ` Willy Tarreau
2022-03-21 11:36     ` Ammar Faizi
2022-03-21 11:43       ` Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).