[RFC PATCH liburing v1 0/2] Fix memset() issue and simplify function naming

[RFC PATCH liburing v1 0/2] Fix memset() issue and simplify function naming

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

On top of "remove useless branches" series. This is an RFC for
liburing nolibc. There are two patches in this series.

## Patch 1: A fix for memset() issue.

liburing has its own memset() in nolibc.c. liburing nolibc can be
linked to apps that use libc. libc has an optimized version of
memset() function. Alviro reports that he found the memset() from
liburing replaces the optimized memset() from libc when he compiled
liburing statically. When we statically link liburing, the linker will
choose the statically linked memset() over the dynamically linked
memset() that the libc provides.

Change the function name to __uring_memset() and define a macro
memset() as:

    #define memset(PTR, C, LEN) __uring_memset(PTR, C, LEN)

when CONFIG_NOLIBC is enabled. This we don't have to touch the
memset() callers.


## Patch 2: Simplify function naming.

Define malloc() and free() as __uring_malloc() and __uring_free() with
macros when CONFIG_NOLIBC is enabled. This way the callers will just
use malloc() and free() instead of uring_malloc() and uring_free().

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (2):
  nolibc: Do not define `memset()` function in liburing
  nolibc: Simplify function naming

 src/lib.h    | 21 +++++----------------
 src/nolibc.c |  2 +-
 src/setup.c  |  6 +++---
 3 files changed, 9 insertions(+), 20 deletions(-)


base-commit: 8fc22e3b3348c0a6384ec926e0b19b6707622e58
prerequisite-patch-id: d74c76e906701902456e2b19f23c100f38f13326
prerequisite-patch-id: bd6f97f77c8f99bd374cde916f97d6223bcbfa33
prerequisite-patch-id: 7c0f399d75e806786b8a7da4d6e23bdb62876710
prerequisite-patch-id: c087dd983f1732fcb9aad8e5b20baf8b9350a935
prerequisite-patch-id: f22f5b7bf9443839ee5bdb5a162c7c7c723a87eb
-- 
Ammar Faizi

[RFC PATCH liburing v1 1/2] nolibc: Do not define `memset()` function in liburing

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

liburing has its own memset() in nolibc.c. liburing nolibc can be
linked to apps that use libc. libc has an optimized version of memset()
function.

Alviro reports that he found the memset() from liburing replaces the
optimized memset() from libc when he compiled liburing statically.

A simple reproducer:

  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>

  int main(void)
  {
          static const size_t len = 1024ul  1024ul  1024ul * 4ul;
          char *p;

          p = malloc(len);
          __asm__ volatile ("":"+m"(p));
          memset(p, 0, len);
          __asm__ volatile ("":"+m"(p));
          return 0;
  }

Compile liburing with:

  # Build liburing nolibc.
  ./configure --nolibc;
  make -j8;

  # Without liburing, memset() comes from libc (good)
  gcc x.c -o x;
  objdump -d x;

  # With liburing.a, memset() comes from liburing (bad)
  gcc x.c -o x src/liburing.a;
  objdump -d x;

When we statically link liburing, the linker will choose the statically
linked memset() over the dynamically linked memset() that the libc
provides.

Change the function name to __uring_memset() and define a macro
memset() as:

   #define memset(PTR, C, LEN) __uring_memset(PTR, C, LEN)

when CONFIG_NOLIBC is enabled so we don't have to touch the callers.

Fixes: f48ee3168cdc325233825603269f304d348d323c ("Add nolibc build support")
Reported-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/lib.h    | 5 +++++
 src/nolibc.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/lib.h b/src/lib.h
index f347191..5a9b87c 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -37,6 +37,7 @@
 #define __hot			__attribute__((__hot__))
 #define __cold			__attribute__((__cold__))
 
+void *__uring_memset(void *s, int c, size_t n);
 void *__uring_malloc(size_t len);
 void __uring_free(void *p);
 
@@ -58,4 +59,8 @@ static inline void uring_free(void *ptr)
 #endif
 }
 
+#ifdef CONFIG_NOLIBC
+#define memset(PTR, C, LEN)	__uring_memset(PTR, C, LEN)
+#endif
+
 #endif /* #ifndef LIBURING_LIB_H */
diff --git a/src/nolibc.c b/src/nolibc.c
index 9a04ead..3207e33 100644
--- a/src/nolibc.c
+++ b/src/nolibc.c
@@ -7,7 +7,7 @@
 #include "lib.h"
 #include "syscall.h"
 
-void *memset(void *s, int c, size_t n)
+void *__uring_memset(void *s, int c, size_t n)
 {
 	size_t i;
 	unsigned char *p = s;
-- 
Ammar Faizi

[RFC PATCH liburing v1 2/2] nolibc: Simplify function naming

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Define malloc() and free() as __uring_malloc() and __uring_free() with
macros when CONFIG_NOLIBC is enabled. This way the callers will just
use malloc() and free() instead of uring_malloc() and uring_free().

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/lib.h   | 22 +++-------------------
 src/setup.c |  6 +++---
 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/src/lib.h b/src/lib.h
index 5a9b87c..a3081da 100644
--- a/src/lib.h
+++ b/src/lib.h
@@ -37,29 +37,13 @@
 #define __hot			__attribute__((__hot__))
 #define __cold			__attribute__((__cold__))
 
+#ifdef CONFIG_NOLIBC
 void *__uring_memset(void *s, int c, size_t n);
 void *__uring_malloc(size_t len);
 void __uring_free(void *p);
 
-static inline void *uring_malloc(size_t len)
-{
-#ifdef CONFIG_NOLIBC
-	return __uring_malloc(len);
-#else
-	return malloc(len);
-#endif
-}
-
-static inline void uring_free(void *ptr)
-{
-#ifdef CONFIG_NOLIBC
-	__uring_free(ptr);
-#else
-	free(ptr);
-#endif
-}
-
-#ifdef CONFIG_NOLIBC
+#define malloc(LEN)		__uring_malloc(LEN)
+#define free(PTR)		__uring_free(PTR)
 #define memset(PTR, C, LEN)	__uring_memset(PTR, C, LEN)
 #endif
 
diff --git a/src/setup.c b/src/setup.c
index d918f86..324f76b 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -215,7 +215,7 @@ __cold struct io_uring_probe *io_uring_get_probe_ring(struct io_uring *ring)
 	int r;
 
 	len = sizeof(*probe) + 256 * sizeof(struct io_uring_probe_op);
-	probe = uring_malloc(len);
+	probe = malloc(len);
 	if (!probe)
 		return NULL;
 	memset(probe, 0, len);
@@ -224,7 +224,7 @@ __cold struct io_uring_probe *io_uring_get_probe_ring(struct io_uring *ring)
 	if (r >= 0)
 		return probe;
 
-	uring_free(probe);
+	free(probe);
 	return NULL;
 }
 
@@ -245,7 +245,7 @@ __cold struct io_uring_probe *io_uring_get_probe(void)
 
 __cold void io_uring_free_probe(struct io_uring_probe *probe)
 {
-	uring_free(probe);
+	free(probe);
 }
 
 static inline int __fls(unsigned long x)
-- 
Ammar Faizi

Re: [RFC PATCH liburing v1 0/2] Fix memset() issue and simplify function naming

[Jens Axboe]

On Thu, 24 Nov 2022 12:46:14 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Hi Jens,
> 
> On top of "remove useless branches" series. This is an RFC for
> liburing nolibc. There are two patches in this series.
> 
> [...]

Applied, thanks!

[1/2] nolibc: Do not define `memset()` function in liburing
      commit: db5403e58083bef48d72656d7dea53a9f7affef4
[2/2] nolibc: Simplify function naming
      commit: 0afc00bfb94121d0642c13d060b63cc07614da17

Best regards,
-- 
Jens Axboe