Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8 bytes size class in tcmalloc and jemalloc breaks ABI on x86_64 #724

Open
spotrh opened this issue Sep 30, 2015 · 13 comments
Open

8 bytes size class in tcmalloc and jemalloc breaks ABI on x86_64 #724

spotrh opened this issue Sep 30, 2015 · 13 comments

Comments

@spotrh
Copy link

spotrh commented Sep 30, 2015

This test program:

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

int
main(void)
{
  printf("max_align_t alignment: %zu\n", _Alignof(max_align_t));
  for (int i = 0; i < 10; ++i) {
    printf("malloc: %p\n", malloc(1));
  }
}

prints:

max_align_t alignment: 16
malloc: 0x23b6010
malloc: 0x23b6018
malloc: 0x23b6020
malloc: 0x23b6028
malloc: 0x23b6030
malloc: 0x23b6038
malloc: 0x23b6040
malloc: 0x23b6048
malloc: 0x23b6050
malloc: 0x23b6058

This means that not all allocations are aligned to 16 bytes, as required by the
x86_64 ABI.

@spotrh
Copy link
Author

spotrh commented Sep 30, 2015

Reported by Florian Weimer in https://bugzilla.redhat.com/show_bug.cgi?id=1237260

@alk
Copy link
Contributor

alk commented Sep 30, 2015

thanks for reporting it.

This is interesting. My understanding is that it's pointless to align less than 16 bytes allocations on 16 bytes.

And that is seemingly position taken by prior maintainers of tcmalloc. Size class of 8 bytes (which is used for all allocations equal or smaller than 8 bytes) is aligned on 8 bytes. And all other sizes classes are aligned on 16 bytes or more.

@alk
Copy link
Contributor

alk commented Sep 30, 2015

And this is actually tested by this line:

if (size >= kMinAlign) {

kMinAlign is defined here:

static const size_t kMinAlign = 16;
(there is TCMALLOC_ALIGN_8BYTES define that is off by default that makes it use 8 bytes minimal alignment)

@alk
Copy link
Contributor

alk commented Sep 30, 2015

So perhaps there is indeed "de jure" requirement of x86-64 ABI to always align even small allocations on 16 bytes. But in practice both tcmalloc and jemalloc (I've just tested your program on jemalloc too) do not align smallest allocations on 16 bytes and that seems 100% reasonably.

Can you, please, point me to "de jure" thing in ABI document anywhere?

@alk
Copy link
Contributor

alk commented Sep 30, 2015

So i've went through http://www.x86-64.org/documentation_folder/abi.pdf searching for places that mention alignment and only requirement that I found that affects malloc is that struct or array alignments are max alignments of elements. Given that no element smaller than 16 bytes has natural alignment of 16 bytes, I believe both tcmalloc and jemalloc are perfectly ABI compliant to align smaller allocations on 8 bytes.

With that closing as 'working as intended'. Feel free to reopen if you have arguments against this.

@fweimer
Copy link

fweimer commented Oct 1, 2015

The de jure ruling is this: The alignment of the type max_align_t is the alignment supported in all contexts, see 6.2.8(2). This includes memory allocated by malloc. The standard does not have a loophole which says that smaller objects do not need to support larger alignment.

On x86_64, _Alignof(max_align_t) is 16. (This is the implementation consensus, max_align_t is younger than the ABI.) Therefore, malloc must return pointers aligned to 16 bytes, even for small allocations.

@alk
Copy link
Contributor

alk commented Oct 1, 2015

Hi.

I disagree about "must align to 16 bytes, even for small allocations".

I read this as follows:

  • "supported" in your text above does not imply, "must always use largest alignment"
  • so the fact that all suitable allocations are aligned on 16 bytes is in my view enough to tick off "supported"
  • that we (and a ton of other malloc implementations) take advantage of smaller alignment for small objects, while not explicitly mentioned by standard, is not forbidden, as far as I can see.

@fweimer
Copy link

fweimer commented Oct 1, 2015

malloc does not know how the pointer is going to be used. The intention of the standard is to say if you have a fundamental alignment (that, less than or equal to that of max_align_t), then it is supported for pointers returned by malloc.

There are basically two ways out: Reduce the alignment of max_align_t to 8 or 4, or clarify the standard that malloc does not have to support fundamental alignment for objects smaller than fundamental alignment. There is some related discussion here:

@alk
Copy link
Contributor

alk commented Oct 1, 2015

Indeed that statement in standard about malloc could be interpreted as "malloc has to align to 16 bytes".

But do note that your example from link above produces struct S that both requires 16 bytes alignment and has sizeof(struct S) == 16. So malloc(sizeof(struct S)) still works correctly.

I'm not a language lawyer, but it seems unreasonable and weird for standard to support something like that:

struct S {
    _Alignas (max_align_t) char a;
};

struct S *foo(void)
{
  return malloc(offsetof(struct S, a) + sizeof(char));
}

So perhaps some other statements in standard effectively forbid anything that could get smaller alignment from malloc and not be OK with it.

@alk
Copy link
Contributor

alk commented Oct 1, 2015

Also I think ultimate intention of standard is something like: "malloc works for malloc(sizeof(T)) of all types" and status quo supports that as far as I can see.

@alk
Copy link
Contributor

alk commented Oct 1, 2015

Correction: "malloc works for malloc(sizeof(T)) for all types that don't have extended alignment".

@fweimer
Copy link

fweimer commented Oct 12, 2015

You are correct that example I posted is invalid, However, It's possible to create a type which shows this problem with a GCC extension (alignment specifier on a typedef).

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

typedef char A __attribute__((aligned(16)));

int
main(void)
{
  printf("malloc(%zu): %p (alignment needed: %zu)\n",
         sizeof(A), malloc(sizeof(A)), __alignof__(A));
}

@alk
Copy link
Contributor

alk commented Oct 12, 2015

On Mon, Oct 12, 2015 at 4:27 AM, Florian Weimer notifications@github.com
wrote:

You are correct that example I posted is invalid, However, It's possible
to create a type which shows this problem with a GCC extension (alignment
specifier on a typedef).

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

typedef char A attribute((aligned(16)));

int
main(void)
{
printf("malloc(%zu): %p (alignment needed: %zu)\n",
sizeof(A), malloc(sizeof(A)), alignof(A));
}

Hi. We still offer aligned_alloc and friends for such cases. Also note that
for 32-bit programs it would require aligned_alloc even with glibc's malloc
that aligns on two words.

For this particular case I think use of gcc extension invalidates
requirements of standard. So for this example status quo means even more.
IMHO.

@alk alk changed the title tcmalloc breaks ABI on x86_64 8 bytes size class in tcmalloc and jemalloc breaks ABI on x86_64 Jun 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants