X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
X-Recipient: djgpp-workers AT delorie DOT com
Message-ID: <529DEE7C.6060508@gmx.de>
Date: Tue, 03 Dec 2013 15:45:16 +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: Re: Wrong order of memalign arguments
References: <5293DBB3 DOT 6020501 AT gmx DOT de>
In-Reply-To: <5293DBB3.6020501@gmx.de>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
X-Provags-ID:  V03:K0:HjQWbiz3g0YeZ4AHmW3tI1Al7pROYRgsvoGO/BzZ2KgkhmZQLxS
 K72Sy4/Ml9JCkdy7vPFflv26E+J3HrU0HtXzaMEvO6c5tJKofPWCxPXh3pBL7ig/84yz2Km
 1mS6GpodNi14S/ysIyWOl33LCEqmfvx5y01jpsAHvGK9n/Xxdp5x3A1xwbAmShY3lN4WDU/
 +7C7nJ5wRxAEsA2NIKwig==
Reply-To: djgpp-workers AT delorie DOT com

Am 26.11.2013 00:22, schrieb Juan Manuel Guerrero:
> 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.
>

OFYI, I have committed the following patch.

Regards,
Juan M. Guerrero



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

     * djgpp/src/libc/compat/stdlib/memalign.txh:  Info about memalign added.


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 insread 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/docs/kb/wc204.txi djgpp/src/docs/kb/wc204.txi
--- djgpp.orig/src/docs/kb/wc204.txi    2013-11-16 21:48:26 +0100
+++ djgpp/src/docs/kb/wc204.txi    2013-11-26 20:24:40 +0100
@@ -1355,5 +1355,16 @@ the @acronym{POSIX} 1003.1-2008 standard
  @findex isinfl AT r{, added to the math library}
  @findex isnanl AT r{, added to the math library}
  @findex finitel AT r{, added to the math library}
  Although they are obsolete, the @acronym{BSD/GNU} compatibility functions @code{isinfl},
  @code{isnanl}, and @code{finitel} were added.
+
+@cindex @acronym{C99} compliance, @file{stdlib.h}
+@findex memalign AT r{, and argument order}
+@findex valloc AT r{, and argument order}
+A bug was fixed in @code{memalign} and @code{valloc}.
+@code{memalign} used to expects its two arguments in the reverse order compared
+with linux and other @acronym{POSIX} compliant systems.  This has been changed
+so that the argument order now is the same that on @acronym{POSIX} compliant
+systems.  @code{valloc}, that calls @code{memalign}, has been adjusted accordingly.
+The @code{memalign} declaration has been moved from AT file{stdlib.h} to
+@file{malloc.h} to comply with the @acronym{C99} standard.
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-26 21:08:40 +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 = (size_t)p;
+
+
+  if (i & (pagesize - 1))
+  {
+    printf("Alignment problem: valloc returns %p\n", p);
+    return 1;
+  }
+  else
+    printf("valloc check passed\n");
+
+  return 0;
+}