X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
X-Recipient: djgpp-workers AT delorie DOT com
Message-ID: <5293DBB3.6020501@gmx.de>
Date: Tue, 26 Nov 2013 00:22:27 +0100
From: Juan Manuel Guerrero <juan DOT guerrero AT gmx DOT de>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
MIME-Version: 1.0
To: djgpp-workers AT delorie DOT com
Subject: Wrong order of memalign arguments
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
X-Provags-ID:  V03:K0:at0kz5jMWtKyVTyFlfohVTGC5p4sa8I/Wf1uEutaTUM4WO0LiII
 QsLXy5Ack/LwwKhpLzFqUZ9Q5k9R9X8UICrec1B/CiYOVO733sXUW01Tt/C/bpLtzpfP0L/
 A3N33VsZAIx0Q6oB0TDRpp304V5LEzb8udynNlBYBcu8v6qrhY8Ih+HTvClIQdP18+wt6CR
 g0ZKqO0MDSnDuOA04fwEA==
Reply-To: djgpp-workers AT delorie DOT com

As Rugxulo pointed out a couple of days before there is a very serious bug in
memalign.  It concerns the reversed order of memalign arguments. From the very
first check in of this file, the function arguments were swapped.  I am not
familiar enough with unix and bsd history, so I do not know if the reversed
argument order was ever the right order.  According to my linux man page this
is not the case.
The second issue is if memalign should still be declared in stdlib.h.  AFAIK
newlib does not provide it at all.  It provides posix_memalign instead and
linux declares memalign in malloc.h (or in stdlib.h if _BSD_SOURCE is defined).

The swapped arguments in mealign is a serious bug and must be fixed.  If
the declaration of memalign shall be moved from stdlib.h to malloc.h may
be objectionable.
The patch below provides a fix for memalign and valloc.  Also it provides
small test cases for both functions.
As usual suggestions, objections, comments are welcome.

Regards,
Juan M. Guerrero



2013-11-25  Juan Manuel Guerrero  <juan DOT guerrero AT gmx DOT de>

     * djgpp/tests/libc/compat/malloc/makefile: Added memalign check.

     * djgpp/tests/libc/compat/malloc/memalign.c: Check memalign function.

     * djgpp/tests/libc/compat/stdlib/makefile: Added valloc check.

     * djgpp/tests/libc/compat/stdlib/valloc.c: Check valloc function.


2013-11-24  Juan Manuel Guerrero  <juan DOT guerrero AT gmx DOT de>

     * djgpp/include/malloc.h: memalign declaration added.

     * djgpp/include/stdlib.h: memalign declaration moved to malloc.h.

     * djgpp/src/libc/compat/stdlib/memalign.c: Switch the argument sequence
     in the memalign definition.

     * djgpp/src/libc/compat/stdlib/memalign.txh: Switch the argument sequence
     in the memalign definition and include malloc.h instead of stdlib.h.

     * djgpp/src/libc/compat/stdlib/valloc.c: Switch the argument sequence
     in the memalign call.






diff -aprNU5 djgpp.orig/include/malloc.h djgpp/include/malloc.h
--- djgpp.orig/include/malloc.h    2012-09-23 07:49:10 +0100
+++ djgpp/include/malloc.h    2013-11-24 23:12:22 +0100
@@ -1,5 +1,6 @@
+/* Copyright (C) 2013 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2002 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
  #ifndef __dj_include_malloc_h_
  #define __dj_include_malloc_h_

@@ -17,10 +18,11 @@ extern "C" {
  #ifndef __STRICT_ANSI__

  #ifndef _POSIX_SOURCE

  #include <stdlib.h>
+void *memalign(size_t _align, size_t _amt);

  #endif /* !_POSIX_SOURCE */
  #endif /* !__STRICT_ANSI__ */
  #endif /* !__dj_ENFORCE_ANSI_FREESTANDING */

diff -aprNU5 djgpp.orig/include/stdlib.h djgpp/include/stdlib.h
--- djgpp.orig/include/stdlib.h    2012-09-23 07:49:12 +0100
+++ djgpp/include/stdlib.h    2013-11-24 23:12:22 +0100
@@ -1,5 +1,6 @@
+/* Copyright (C) 2013 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2003 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2002 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2001 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2000 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 1999 DJ Delorie, see COPYING.DJ for details */
@@ -130,11 +131,10 @@ char *        getpass(const char *_prompt);
  int        getlongpass(const char *_prompt, char *_buffer, int _max_len);
  char *        itoa(int _value, char *_buffer, int _radix);
  long            jrand48(unsigned short _state[3]);
  void            lcong48(unsigned short _param[7]);
  unsigned long   lrand48(void);
-void *        memalign (size_t _amt, size_t _align);
  long            mrand48(void);
  unsigned long   nrand48(unsigned short _state[3]);
  unsigned short *seed48(unsigned short _state_seed[3]);
  void            srand48(long _seedval);
  int        stackavail(void);
diff -aprNU5 djgpp.orig/src/libc/compat/stdlib/memalign.c djgpp/src/libc/compat/stdlib/memalign.c
--- djgpp.orig/src/libc/compat/stdlib/memalign.c    2013-11-24 22:17:22 +0100
+++ djgpp/src/libc/compat/stdlib/memalign.c    2013-11-24 23:12:22 +0100
@@ -1,7 +1,8 @@
+/* Copyright (C) 2013 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2001 DJ Delorie, see COPYING.DJ for details */
-#include <stdlib.h>
+#include <malloc.h>
  #include <libc/malloc.h>

  /* Make VAL a multiple of ALIGN.  */
  static inline
  size_t
@@ -65,11 +66,11 @@ split_alloc(char *ptr, size_t split_pos)
  }

  /* Return a block of memory AMT bytes long whose address is a multiple
     ALIGN.  ALIGN must be a power of 2.  */
  void *
-memalign(size_t amt, size_t align)
+memalign(size_t align, size_t amt)
  {
    char *ptr, *aligned_ptr;
    size_t alloc_size, before_size, after_size;

    if (align != align_val(align, align))
diff -aprNU5 djgpp.orig/src/libc/compat/stdlib/memalign.txh djgpp/src/libc/compat/stdlib/memalign.txh
--- djgpp.orig/src/libc/compat/stdlib/memalign.txh    2003-01-29 12:41:10 +0100
+++ djgpp/src/libc/compat/stdlib/memalign.txh    2013-11-24 23:12:22 +0100
@@ -1,13 +1,13 @@
  @node memalign, memory
  @findex memalign
  @subheading Syntax

  @example
-#include <stdlib.h>
+#include <malloc.h>

-void *memalign(size_t size, size_t alignment);
+void *memalign(size_t alignment, size_t size);
  @end example

  @subheading Description

  This function is like @code{malloc} (@pxref{malloc}) except the returned
@@ -23,7 +23,7 @@ A pointer to a newly allocated block of
  @portability !ansi, !posix

  @subheading Example

  @example
-char *page = memalign(1024, 1024 * 4);
+char *page = memalign(1024 * 4, 1024);
  @end example
diff -aprNU5 djgpp.orig/src/libc/compat/stdlib/valloc.c djgpp/src/libc/compat/stdlib/valloc.c
--- djgpp.orig/src/libc/compat/stdlib/valloc.c    2013-11-24 22:15:24 +0100
+++ djgpp/src/libc/compat/stdlib/valloc.c    2013-11-24 23:13:50 +0100
@@ -1,15 +1,16 @@
+/* Copyright (C) 2013 DJ Delorie, see COPYING.DJ for details */
  /* Copyright (C) 2001 DJ Delorie, see COPYING.DJ for details */

-#include <stdlib.h>
+#include <malloc.h>
  #include <unistd.h>

  void *
  valloc(size_t amt)
  {
    static int page_size = -1;

    if (page_size == -1)
      page_size = getpagesize();

-  return memalign(amt, page_size);
+  return memalign(page_size, amt);
  }
diff -aprNU5 djgpp.orig/tests/libc/compat/malloc/makefile djgpp/tests/libc/compat/malloc/makefile
--- djgpp.orig/tests/libc/compat/malloc/makefile    1970-01-01 01:00:00 +0100
+++ djgpp/tests/libc/compat/malloc/makefile    2013-11-25 20:21:54 +0100
@@ -0,0 +1,5 @@
+TOP=../..
+
+SRC += memalign.c
+
+include $(TOP)/../makefile.inc
diff -aprNU5 djgpp.orig/tests/libc/compat/malloc/memalign.c djgpp/tests/libc/compat/malloc/memalign.c
--- djgpp.orig/tests/libc/compat/malloc/memalign.c    1970-01-01 01:00:00 +0100
+++ djgpp/tests/libc/compat/malloc/memalign.c    2013-11-25 22:09:20 +0100
@@ -0,0 +1,50 @@
+#include <stdio.h>
+#include <malloc.h>
+
+int
+main(void)
+{
+  size_t aligment = 0;
+  size_t amount = 1024;
+  char *p;
+  int i, failed = 0;
+
+
+  p =  memalign(aligment, amount);
+  if (!p)
+  {
+    printf("memalign failed to align on byte boundary.\n");
+    failed++;
+  }
+
+  p =  memalign(++aligment, amount);
+  if ((size_t)p % aligment)
+  {
+    printf("memalign erroneously allowed to aligned on %zd byte boundary.\n", aligment);
+    failed++;
+  }
+
+  for (++aligment, i = 0; i < 10; i++, aligment = 2 << i)
+  {
+    p =  memalign(aligment, amount);
+    if (!p || (size_t)p % aligment)
+    {
+      printf("memalign failed to align on %zd byte boundary.\n", aligment);
+      failed++;
+    }
+
+    p =  memalign(++aligment, amount);
+    if (p)
+    {
+      printf("memalign erroneously allowed to aligned on %zd byte boundary.\n", aligment);
+      failed++;
+    }
+  }
+
+  if (failed)
+    printf("memalign failed for %d checks.\n", failed);
+  else
+    printf("memalign check passed.\n");
+
+  return 0;
+}
diff -aprNU5 djgpp.orig/tests/libc/compat/stdlib/makefile djgpp/tests/libc/compat/stdlib/makefile
--- djgpp.orig/tests/libc/compat/stdlib/makefile    1970-01-01 01:00:00 +0100
+++ djgpp/tests/libc/compat/stdlib/makefile    2013-11-25 19:56:00 +0100
@@ -0,0 +1,5 @@
+TOP=../..
+
+SRC += valloc.c
+
+include $(TOP)/../makefile.inc
diff -aprNU5 djgpp.orig/tests/libc/compat/stdlib/valloc.c djgpp/tests/libc/compat/stdlib/valloc.c
--- djgpp.orig/tests/libc/compat/stdlib/valloc.c    1970-01-01 01:00:00 +0100
+++ djgpp/tests/libc/compat/stdlib/valloc.c    2013-11-25 22:09:18 +0100
@@ -0,0 +1,24 @@
+/* Copyright (C) 2013 DJ Delorie, see COPYING.DJ for details */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+main(void)
+{
+  int pagesize = getpagesize();
+  char *p = valloc(pagesize);
+  int i = (long int)p;
+
+
+  if (i & (pagesize - 1))
+  {
+    printf("Alignment problem: valloc returns %p\n", p);
+    return 1;
+  }
+  else
+    printf("valloc check passed\n");
+
+  return 0;
+}