public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Make sure MALLOC_ALIGNMENT is at least long double's alignment
@ 2006-02-28 15:21 Jakub Jelinek
  2006-03-02 15:04 ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2006-02-28 15:21 UTC (permalink / raw)
  To: Ulrich Drepper, Roland McGrath, Wolfram Gloger; +Cc: Glibc hackers

Hi!

The recent switch to 128-bit long double on ppc32 broke malloc,
as on this 32-bit arch long double is 16 byte aligned, but malloc
was only guaranteeing 8 byte aligment so far.
On sparc32 and s390 this is not a problem, since __alignof__ (long double)
is 8.

But, apparently some of the quick checks for invalid pointers don't
work when MALLOC_ALIGNMENT is bigger than 2 * SIZE_SZ, in that case
the pointer returned to the user must be MALLOC_ALIGNMENT aligned,
not the chunk pointer (which is always user pointer - 2 * SIZE_SZ).

With this, ppc32 glibc passes make check.
I haven't checked though if this pessimizes code on i?86/x86_64 or other
important arches, will do soon.  If it does, then we might consider doing:

#define check_align_OK(chunkptr, userptr) \
  (MALLOC_ALIGNMENT > 2 * SIZE_SZ \
   ? __builtin_expect ((uintptr_t) userptr & MALLOC_ALIGN_MASK, 0) \
   : __builtin_expect ((uintptr_t) chunkptr & MALLOC_ALIGN_MASK, 0))

and using check_align_OK (oldp, oldmem) etc.

2006-02-28  Jakub Jelinek  <jakub@redhat.com>

	* malloc/malloc.c (MALLOC_ALIGNMENT): Set to __alignof__ (long double)
	if long double is more aligned than 2 * SIZE_SZ.
	(public_rEALLOc, _int_free, _int_realloc): Check that *mem is aligned,
	rather than corresponding mem2chunk pointer.

--- libc/malloc/malloc.c	2005-12-30 09:04:02.000000000 +0100
+++ libc/malloc/malloc.c	2006-02-28 15:30:20.000000000 +0100
@@ -188,7 +188,8 @@
     Changing default word sizes:
 
     INTERNAL_SIZE_T            size_t
-    MALLOC_ALIGNMENT           2 * sizeof(INTERNAL_SIZE_T)
+    MALLOC_ALIGNMENT           MAX (2 * sizeof(INTERNAL_SIZE_T),
+				    __alignof__ (long double))
 
     Configuration and functionality options:
 
@@ -380,7 +381,8 @@ extern "C" {
 
 
 #ifndef MALLOC_ALIGNMENT
-#define MALLOC_ALIGNMENT       (2 * SIZE_SZ)
+#define MALLOC_ALIGNMENT       (2 * SIZE_SZ < __alignof__ (long double) \
+				? __alignof__ (long double) : 2 * SIZE_SZ)
 #endif
 
 /* The corresponding bit mask value */
@@ -3468,7 +3470,7 @@ public_rEALLOc(Void_t* oldmem, size_t by
      Therefore we can exclude some size values which might appear
      here by accident or by "design" from some intruder.  */
   if (__builtin_expect ((uintptr_t) oldp > (uintptr_t) -oldsize, 0)
-      || __builtin_expect ((uintptr_t) oldp & MALLOC_ALIGN_MASK, 0))
+      || __builtin_expect ((uintptr_t) oldmem & MALLOC_ALIGN_MASK, 0))
     {
       malloc_printerr (check_action, "realloc(): invalid pointer", oldmem);
       return NULL;
@@ -4282,7 +4284,7 @@ _int_free(mstate av, Void_t* mem)
      Therefore we can exclude some size values which might appear
      here by accident or by "design" from some intruder.  */
   if (__builtin_expect ((uintptr_t) p > (uintptr_t) -size, 0)
-      || __builtin_expect ((uintptr_t) p & MALLOC_ALIGN_MASK, 0))
+      || __builtin_expect ((uintptr_t) mem & MALLOC_ALIGN_MASK, 0))
     {
       errstr = "free(): invalid pointer";
     errout:
@@ -4628,7 +4630,7 @@ _int_realloc(mstate av, Void_t* oldmem, 
   oldsize = chunksize(oldp);
 
   /* Simple tests for old block integrity.  */
-  if (__builtin_expect ((uintptr_t) oldp & MALLOC_ALIGN_MASK, 0))
+  if (__builtin_expect ((uintptr_t) oldmem & MALLOC_ALIGN_MASK, 0))
     {
       errstr = "realloc(): invalid pointer";
     errout:

	Jakub

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

* Re: [PATCH] Make sure MALLOC_ALIGNMENT is at least long double's alignment
  2006-02-28 15:21 [PATCH] Make sure MALLOC_ALIGNMENT is at least long double's alignment Jakub Jelinek
@ 2006-03-02 15:04 ` Jakub Jelinek
  2006-03-02 15:53   ` Ulrich Drepper
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2006-03-02 15:04 UTC (permalink / raw)
  To: Roland McGrath, Ulrich Drepper; +Cc: Wolfram Gloger, Glibc hackers

Hi!

On Tue, Feb 28, 2006 at 04:21:00PM +0100, Jakub Jelinek wrote:
> The recent switch to 128-bit long double on ppc32 broke malloc,
> as on this 32-bit arch long double is 16 byte aligned, but malloc
> was only guaranteeing 8 byte aligment so far.
> On sparc32 and s390 this is not a problem, since __alignof__ (long double)
> is 8.

Here is an updated patch that results in no code changes unless
MALLOC_ALIGNMENT is bigger than the 2 * SIZE_SZ default (so only on ppc32):

2006-03-02  Jakub Jelinek  <jakub@redhat.com>

	* malloc/malloc.c (MALLOC_ALIGNMENT): Set to __alignof__ (long double)
	if long double is more aligned than 2 * SIZE_SZ.
	(misaligned_chunk): Define.
	(public_rEALLOc, _int_free, _int_realloc): Use it.

--- libc/malloc/malloc.c.jj	2005-12-30 09:04:02.000000000 +0100
+++ libc/malloc/malloc.c	2006-03-02 10:52:38.000000000 +0100
@@ -188,7 +188,8 @@
     Changing default word sizes:
 
     INTERNAL_SIZE_T            size_t
-    MALLOC_ALIGNMENT           2 * sizeof(INTERNAL_SIZE_T)
+    MALLOC_ALIGNMENT           MAX (2 * sizeof(INTERNAL_SIZE_T),
+				    __alignof__ (long double))
 
     Configuration and functionality options:
 
@@ -380,7 +381,8 @@ extern "C" {
 
 
 #ifndef MALLOC_ALIGNMENT
-#define MALLOC_ALIGNMENT       (2 * SIZE_SZ)
+#define MALLOC_ALIGNMENT       (2 * SIZE_SZ < __alignof__ (long double) \
+				? __alignof__ (long double) : 2 * SIZE_SZ)
 #endif
 
 /* The corresponding bit mask value */
@@ -1807,7 +1809,11 @@ nextchunk-> +-+-+-+-+-+-+-+-+-+-+-+-+-+-
 
 /* Check if m has acceptable alignment */
 
-#define aligned_OK(m)  (((unsigned long)((m)) & (MALLOC_ALIGN_MASK)) == 0)
+#define aligned_OK(m)  (((unsigned long)(m) & MALLOC_ALIGN_MASK) == 0)
+
+#define misaligned_chunk(p) \
+  ((uintptr_t)(MALLOC_ALIGNMENT == 2 * SIZE_SZ ? (p) : chunk2mem (p)) \
+   & MALLOC_ALIGN_MASK)
 
 
 /*
@@ -3468,7 +3474,7 @@ public_rEALLOc(Void_t* oldmem, size_t by
      Therefore we can exclude some size values which might appear
      here by accident or by "design" from some intruder.  */
   if (__builtin_expect ((uintptr_t) oldp > (uintptr_t) -oldsize, 0)
-      || __builtin_expect ((uintptr_t) oldp & MALLOC_ALIGN_MASK, 0))
+      || __builtin_expect (misaligned_chunk (oldp), 0))
     {
       malloc_printerr (check_action, "realloc(): invalid pointer", oldmem);
       return NULL;
@@ -4282,7 +4288,7 @@ _int_free(mstate av, Void_t* mem)
      Therefore we can exclude some size values which might appear
      here by accident or by "design" from some intruder.  */
   if (__builtin_expect ((uintptr_t) p > (uintptr_t) -size, 0)
-      || __builtin_expect ((uintptr_t) p & MALLOC_ALIGN_MASK, 0))
+      || __builtin_expect (misaligned_chunk (p), 0))
     {
       errstr = "free(): invalid pointer";
     errout:
@@ -4628,7 +4634,7 @@ _int_realloc(mstate av, Void_t* oldmem, 
   oldsize = chunksize(oldp);
 
   /* Simple tests for old block integrity.  */
-  if (__builtin_expect ((uintptr_t) oldp & MALLOC_ALIGN_MASK, 0))
+  if (__builtin_expect (misaligned_chunk (oldp), 0))
     {
       errstr = "realloc(): invalid pointer";
     errout:


	Jakub

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

* Re: [PATCH] Make sure MALLOC_ALIGNMENT is at least long double's alignment
  2006-03-02 15:04 ` Jakub Jelinek
@ 2006-03-02 15:53   ` Ulrich Drepper
  0 siblings, 0 replies; 3+ messages in thread
From: Ulrich Drepper @ 2006-03-02 15:53 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Wolfram Gloger, Glibc hackers

[-- Attachment #1: Type: text/plain, Size: 101 bytes --]

Applied.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

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

end of thread, other threads:[~2006-03-02 15:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-02-28 15:21 [PATCH] Make sure MALLOC_ALIGNMENT is at least long double's alignment Jakub Jelinek
2006-03-02 15:04 ` Jakub Jelinek
2006-03-02 15:53   ` Ulrich Drepper

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 read-only IMAP folder(s) and NNTP newsgroup(s).