public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ammar Faizi <[email protected]>
To: Willy Tarreau <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>,
	Alviro Iskandar Setiawan <[email protected]>,
	Nugraha <[email protected]>,
	Linux Kernel Mailing List <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>
Subject: Re: [RFC PATCH v1 5/6] tools/nolibc/stdlib: Implement `malloc()`, `calloc()`, `realloc()` and `free()`
Date: Sun, 20 Mar 2022 23:36:55 +0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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

  reply	other threads:[~2022-03-20 16:36 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox