Re: memcmp.c needs intptr_t

[Bruno Haible]

Hello Ozkan,

> Hello:  Found this while compiling gdb for win64: memcmp.c
> casts long to pointers but sizeof(long) == 4 for win64. A patch
> is attached here.

Thanks for reporting this. Indeed there are two things wrong in
gnulib's memcmp.c:
  - The assumption that sizeof (long) >= sizeof (void *), not valid
    in the Windows64 programming model,
  - The assumption that a 'long int' or 'intptr_t' can be arbitrarily
    incremented. Not true in C99: it can lead to undefined behaviour,
    i.e. to program crashes. The fix is to used unsigned instead of
    signed integer types.

Your patch fixes the first problem, but the part concerning memcmp_bytes
is very confusing. (Look how memcmp_bytes is used.)

I'm fixing gnulib's memcmp.c below.

Note that it relies on a valid <stdint.h> file - but we know that
Windows does not provide <stdint.h>, and gnulib's replacement does
not yet work for sizeof (long) < sizeof (void *).

> (if it gets applied, does it automatically go 
> into gdb and/or any other users?)

Fixes in gnulib go automatically into many projects, from bison to
zile. But gdb and gcc use libiberty, not gnulib. Traditionally,
libiberty has been a Cygnus (now RedHat) project, whereas gnulib
is a GNU project.

> PS:  There are a few similar errors of long type in libiberty,
> as it appears in binutils and gcc, too, specifically in md5.h
> (uintptr_t type definition) and in hashtab.c (in hash_pointer() ).
> Where should the patches be sent to?

Patches to the libiberty that you find in gdb or binutils should go
to bug-binutils.

Patches to the libiberty that you find in gcc should go to
http://gcc.gnu.org/bugzilla/

Patches to gnulib should go to bug-gnulib.

Bruno


2009-02-01  Bruno Haible  <address@hidden>

        Don't assume sizeof (long) >= sizeof (void *).
        * lib/memcmp.c: Include stdint.h.
        (memcmp_bytes): Change argument types to op_t. Change type of srcp1,
        srcp2 to 'const byte *'.
        (memcmp_common_alignment, memcmp_not_common_alignment): Change argument
        types to uintptr_t.
        (rpl_memcmp): Change type of srcp1, srcp2 to 'uintptr_t'.
        * modules/memcmp (Depends-on): Add stdint.
        Reported by Ozkan Sezer <address@hidden>.

--- lib/memcmp.c.orig   2009-02-01 21:56:54.000000000 +0100
+++ lib/memcmp.c        2009-02-01 21:55:59.000000000 +0100
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991, 1993, 1995, 1997, 1998, 2003, 2006 Free Software
+/* Copyright (C) 1991, 1993, 1995, 1997, 1998, 2003, 2006, 2009 Free Software
    Foundation, Inc.
 
    Contributed by Torbjorn Granlund (address@hidden).
@@ -25,6 +25,8 @@
 
 #include <string.h>
 
+#include <stdint.h>
+
 #undef memcmp
 
 #ifdef _LIBC
@@ -88,16 +90,16 @@
 __inline
 # endif
 static int
-memcmp_bytes (long unsigned int a, long unsigned int b)
+memcmp_bytes (op_t a, op_t b)
 {
-  long int srcp1 = (long int) &a;
-  long int srcp2 = (long int) &b;
+  const byte *srcp1 = (const byte *) &a;
+  const byte *srcp2 = (const byte *) &b;
   op_t a0, b0;
 
   do
     {
-      a0 = ((byte *) srcp1)[0];
-      b0 = ((byte *) srcp2)[0];
+      a0 = srcp1[0];
+      b0 = srcp2[0];
       srcp1 += 1;
       srcp2 += 1;
     }
@@ -113,7 +115,7 @@
 __inline
 #endif
 static int
-memcmp_common_alignment (long int srcp1, long int srcp2, size_t len)
+memcmp_common_alignment (uintptr_t srcp1, uintptr_t srcp2, size_t len)
 {
   op_t a0, a1;
   op_t b0, b1;
@@ -198,7 +200,7 @@
 __inline
 #endif
 static int
-memcmp_not_common_alignment (long int srcp1, long int srcp2, size_t len)
+memcmp_not_common_alignment (uintptr_t srcp1, uintptr_t srcp2, size_t len)
 {
   op_t a0, a1, a2, a3;
   op_t b0, b1, b2, b3;
@@ -302,8 +304,8 @@
 {
   op_t a0;
   op_t b0;
-  long int srcp1 = (long int) s1;
-  long int srcp2 = (long int) s2;
+  uintptr_t srcp1 = (uintptr_t) s1;
+  uintptr_t srcp2 = (uintptr_t) s2;
   op_t res;
 
   if (len >= OP_T_THRES)
--- modules/memcmp.orig 2009-02-01 21:56:54.000000000 +0100
+++ modules/memcmp      2009-02-01 21:54:44.000000000 +0100
@@ -12,6 +12,7 @@
 m4/memcmp.m4
 
 Depends-on:
+stdint
 
 configure.ac:
 gl_FUNC_MEMCMP