[PATCH v2 0/9] Add dynamic memory allocator support for nolibc

Ammar Faizi posted 9 patches 4 years, 2 months ago
tools/include/nolibc/arch-aarch64.h | 75 +++++++++++++-------------
tools/include/nolibc/arch-arm.h     | 59 +++++++++++----------
tools/include/nolibc/arch-i386.h    | 80 +++++++++++++++++-----------
tools/include/nolibc/arch-mips.h    | 63 +++++++++++-----------
tools/include/nolibc/arch-riscv.h   | 75 +++++++++++++-------------
tools/include/nolibc/arch-x86_64.h  | 75 +++++++++++++-------------
tools/include/nolibc/stdlib.h       | 81 +++++++++++++++++++++++++++++
tools/include/nolibc/string.h       | 41 +++++++++++++++
tools/include/nolibc/sys.h          | 62 ++++++++++++++++++++++
tools/include/nolibc/types.h        | 11 ++++
10 files changed, 417 insertions(+), 205 deletions(-)
[PATCH v2 0/9] Add dynamic memory allocator support for nolibc
Posted by Ammar Faizi 4 years, 2 months ago
Hi,

This is a patchset v2 to add dynamic memory allocator support
for nolibc after 2 RFCs, please review the changes carefully.

@@ Changelog:
---
   Link v1: https://lore.kernel.org/lkml/20220324073039.140946-1-ammarfaizi2@gnuweeb.org
   v1 -> v2:
     - Sync with Paul's tree.
     - Drop 2 patches that tried to remove register variables
       (comment from Willy).
     - Make the patch that replaces `asm` with `__asm__` the
       second patch (comment from Willy).

   Link RFC v2: https://lore.kernel.org/lkml/20220322102115.186179-1-ammarfaizi2@gnuweeb.org
   RFC v2 -> v1:
     - Rebase, sync with Paul's tree.
     - Add new 3 patches [PATCH 03/11], [PATCH 04/11], [PATCH 05/11].

     [PATCH 02/11]
     - Append Reviewed-by tag from Nick.
     - s/Removing/remove/

     [PATCH 06/11]
     - Use the same pattern for syscall6, regardless using GCC or Clang
      (comment from David).
     - Use appropriate constraints for syscall6 instead of always using
      register variables (comment from David).

     [PATCH 09/11]
     - Round up the malloc() allocation to 4096 (comment from David).
     - Don't realloc() if we still have enough memory to contain the
       requested new size (comment from David).
     - Fix conflict with getenv() fix (after rebase).

   Link RFC v1: https://lore.kernel.org/lkml/20220320093750.159991-1-ammarfaizi2@gnuweeb.org
   RFC v1 -> RFC v2:
    - Add 2 new patches [PATCH 5/8] and [PATCH 7/8].

    [PATCH 2/8]
    - Remove all `.global _start` for all build (GCC and Clang) instead of
      removing all `.weak _start` for clang build (Comment from Willy).

    [PATCH 3/8]
    - Fix %ebp saving method. Don't use redzone, i386 doesn't have a redzone
      (comment from David and Alviro).

    [PATCH 6/8]
    - Move container_of() and offsetof() macro to types.h with a
      separate patch (comment from Willy).

    [PATCH 8/8]
    - Update strdup and strndup implementation, use strlen and strnlen to get
      the string length first (comment from Willy and Alviro).
    - Fix the subject line prefix, it was "tools/include/string: ", it should be
      "tools/nolibc/string: ".
    - Update the commit message.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
Ammar Faizi (9):
  tools/nolibc: x86-64: Update System V ABI document link
  tools/nolibc: Replace `asm` with `__asm__`
  tools/nolibc: Remove .global _start from the entry point code
  tools/nolibc: i386: Implement syscall with 6 arguments
  tools/nolibc/sys: Implement `mmap()` and `munmap()`
  tools/nolibc/types: Implement `offsetof()` and `container_of()` macro
  tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
  tools/nolibc/string: Implement `strnlen()`
  tools/include/string: Implement `strdup()` and `strndup()`

 tools/include/nolibc/arch-aarch64.h | 75 +++++++++++++-------------
 tools/include/nolibc/arch-arm.h     | 59 +++++++++++----------
 tools/include/nolibc/arch-i386.h    | 80 +++++++++++++++++-----------
 tools/include/nolibc/arch-mips.h    | 63 +++++++++++-----------
 tools/include/nolibc/arch-riscv.h   | 75 +++++++++++++-------------
 tools/include/nolibc/arch-x86_64.h  | 75 +++++++++++++-------------
 tools/include/nolibc/stdlib.h       | 81 +++++++++++++++++++++++++++++
 tools/include/nolibc/string.h       | 41 +++++++++++++++
 tools/include/nolibc/sys.h          | 62 ++++++++++++++++++++++
 tools/include/nolibc/types.h        | 11 ++++
 10 files changed, 417 insertions(+), 205 deletions(-)


base-commit: 2eb9d6a49acd4f12078967c33e9786e084fe6407
-- 
Ammar Faizi
Re: [PATCH v2 0/9] Add dynamic memory allocator support for nolibc
Posted by Ammar Faizi 4 years, 2 months ago
On 3/29/22 5:17 PM, Ammar Faizi wrote:
> Hi,
> 
> This is a patchset v2 to add dynamic memory allocator support
> for nolibc after 2 RFCs, please review the changes carefully.

Sorry, forgot to replace === with --- in for each patch.
Should I resend?

-- 
Ammar Faizi
Re: [PATCH v2 0/9] Add dynamic memory allocator support for nolibc
Posted by Willy Tarreau 4 years, 2 months ago
Hi Ammar,

On Tue, Mar 29, 2022 at 05:20:31PM +0700, Ammar Faizi wrote:
> On 3/29/22 5:17 PM, Ammar Faizi wrote:
> > Hi,
> > 
> > This is a patchset v2 to add dynamic memory allocator support
> > for nolibc after 2 RFCs, please review the changes carefully.

Thank you! For me it's OK for all the series:

Acked-by: Willy Tarreau <w@1wt.eu>

I do have a minor comment about the use of __builtin_mul_overflow() here.
While this code is included in the kernel and mostly for use with kernel
related stuff, till now I've been careful to support older compilers (I'm
still seeing 4.8, 4.7 and 4.4 commonly in field). I don't find it urgent,
but I think that sooner or later it would be nice to implement an
alternative for compilers missing this builtin, especially if it's the
only one that prevents older compilers from being used. Probably that
something like this (untested) would do the job:

	if (nmemb && ~(size_t)0 / nmemb < size) {
		SET_ERRNO(ENOMEM);
		return NULL;
	}
	size *= nmemb;

But again, for me it's not a showstopper and can be improved later.

> Sorry, forgot to replace === with --- in for each patch.
> Should I resend?

Let's see what Paul prefers. sed 's/===/---/' on the mbox should fix
it otherwise a resend will be needed.

Thanks!
Willy
Re: [PATCH v2 0/9] Add dynamic memory allocator support for nolibc
Posted by Paul E. McKenney 4 years, 2 months ago
On Wed, Mar 30, 2022 at 04:41:14AM +0200, Willy Tarreau wrote:
> Hi Ammar,
> 
> On Tue, Mar 29, 2022 at 05:20:31PM +0700, Ammar Faizi wrote:
> > On 3/29/22 5:17 PM, Ammar Faizi wrote:
> > > Hi,
> > > 
> > > This is a patchset v2 to add dynamic memory allocator support
> > > for nolibc after 2 RFCs, please review the changes carefully.
> 
> Thank you! For me it's OK for all the series:
> 
> Acked-by: Willy Tarreau <w@1wt.eu>
> 
> I do have a minor comment about the use of __builtin_mul_overflow() here.
> While this code is included in the kernel and mostly for use with kernel
> related stuff, till now I've been careful to support older compilers (I'm
> still seeing 4.8, 4.7 and 4.4 commonly in field). I don't find it urgent,
> but I think that sooner or later it would be nice to implement an
> alternative for compilers missing this builtin, especially if it's the
> only one that prevents older compilers from being used. Probably that
> something like this (untested) would do the job:
> 
> 	if (nmemb && ~(size_t)0 / nmemb < size) {
> 		SET_ERRNO(ENOMEM);
> 		return NULL;
> 	}
> 	size *= nmemb;
> 
> But again, for me it's not a showstopper and can be improved later.
> 
> > Sorry, forgot to replace === with --- in for each patch.
> > Should I resend?
> 
> Let's see what Paul prefers. sed 's/===/---/' on the mbox should fix
> it otherwise a resend will be needed.

Given that I am adding your Acked-by anyway, why not?  ;-)

But please check the commits to make sure that this had the desired
effect.

							Thanx, Paul
Re: [PATCH v2 0/9] Add dynamic memory allocator support for nolibc
Posted by Willy Tarreau 4 years, 2 months ago
On Wed, Mar 30, 2022 at 11:51:52AM -0700, Paul E. McKenney wrote:
> > > Sorry, forgot to replace === with --- in for each patch.
> > > Should I resend?
> > 
> > Let's see what Paul prefers. sed 's/===/---/' on the mbox should fix
> > it otherwise a resend will be needed.
> 
> Given that I am adding your Acked-by anyway, why not?  ;-)

:-)

> But please check the commits to make sure that this had the desired
> effect.

All of it looks perfect to me, many thanks Paul!

Willy