AC_TYPE_UINT64_T produces wrong result with GCC 3.4.4 on Tru64 UNIX V4.0F

[Rainer Orth-2]

I noticed that the AC_TYPE_UINT64_T macro in autoconf 2.64 produces a
wrong results on Tru64 UNIX V4.0F with GCC 3.4.4.  The platform lacks
both stdint.h and inttypes.h, so the macro needs to provide a value
itself.

Here's the test I used:

$ cat configure.ac
AC_INIT(uint64_t, 0.0)
AC_TYPE_UINT64_T
AC_CONFIG_HEADERS(config.h)
AC_OUTPUT
$ autoconf
$ autoheader
$ ./configure
[...]
checking for inttypes.h... no
checking for stdint.h... no
checking for unistd.h... yes
checking for uint64_t... unsigned int
configure: creating ./config.status
config.status: creating config.h

The value above is wrong: alpha-dec-osf is an LP64 platform, so this
should be unsigned long instead.

The problem is shown with the extracted testcase:

$ cat conftest.c
int
main ()
{
static int test_array [1 - 2 * !((unsigned int) -1 >> (64 - 1) == 1)];
test_array [0] = 0

  ;
  return 0;
}
$ gcc-3.4.4 -c conftest.c
conftest.c: In function `main':
conftest.c:4: warning: right shift count >= width of type
$ echo $?
0
$ gcc-4.5.0 -c conftest.c
conftest.c: In function 'main':
conftest.c:4:1: warning: right shift count >= width of type
conftest.c:4:12: error: storage size of 'test_array' isn't constant
$ echo $?
1

I'm not sure this is a problem in all versions of GCC 3.4 on that
platform or specific to the version I use, though.

        Rainer

--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Rainer Orth on 10/19/2009 9:23 AM:
> checking for uint64_t... unsigned int
> $ gcc-3.4.4 -c conftest.c
> conftest.c: In function `main':
> conftest.c:4: warning: right shift count >= width of type
> $ echo $?
> 0

Thanks for the report.  It is indeed a bug, and we should be able to work
around it.  We need to break x>>63 into the more appropriate
((x>>31)>>31)>>1 to work around this.  But before I work on the necessary
m4 magic, is it also a problem with smaller types?  In other words, what
happens with:

int
main ()
{
static int test_array [1 - 2 * !((unsigned char) -1 >> (32 - 1) == 1)];
test_array [0] = 0

  ;
  return 0;
}

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@...
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrdt1wACgkQ84KuGfSFAYDYBgCdGUVtWQXg86nuIhf+uGpPxPLw
BcMAn3ORB43DHvJPsWkeLd+vuuCRjgl0
=QkCH
-----END PGP SIGNATURE-----

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> According to Rainer Orth on 10/19/2009 9:23 AM:
> > checking for uint64_t... unsigned int
> > $ gcc-3.4.4 -c conftest.c
> > conftest.c: In function `main':
> > conftest.c:4: warning: right shift count >= width of type
> > $ echo $?
> > 0
>
> Thanks for the report.  It is indeed a bug, and we should be able to work
> around it.  We need to break x>>63 into the more appropriate
> ((x>>31)>>31)>>1 to work around this.

Like in the attached patch.  It's hard for me to test, since most systems these
days already have int64_t, so my for loop wasn't getting that far, but by
inspection, it looks like it will solve your problem.

>  But before I work on the necessary
> m4 magic, is it also a problem with smaller types?

On further review, I don't think this is an issue.  Checking a wider candidate
(long) against a narrower type (uint8_t) is not the issue, only a narrower
candidate (int) against a wider type (uint64_t), and using a shift width that
works for the wider type but gives undefined behavior for the narrower type.  
But since we control the order in which types are checked, and since my patch
breaks things up to never shift more than half the target type (that is, for
uint64_t, my patch never shifts more than 31), we can now safely check for a
half-width type first.  That means we don't have to reorder checks, so that we
maintain the current status quo of fewer compilation attempts for uint32_t than
for uint64_t (on the assumption that more configure scripts check for the
former than the latter).

An alternate patch would have been reordering the shell for loop to test long
long, long, then int; that way, the checks are ordered widest to narrowest.  
But that has the potential for API changes if the macro selects a different
type (but equivalent width) than what it did before the reordering, and it also
penalizes the time taken for selecting the type to use for uint32_t.

A potential followup optimization would be updating ac_fn_c_find_intX_t to take
an additional parameter of the C type most likely to work, and inserting that
type into the shell for loop between intX_t and int; that way, finding int8_t
can more quickly check for 'signed char' without first cycling through int,
long, long long, and short.  But like I said, the number of platforms these
days without pre-defined intX_t is getting smaller, so few people would benefit
from such an optimization.


From: Eric Blake <ebb9@...>
Date: Tue, 20 Oct 2009 08:30:03 -0600
Subject: [PATCH] Fix AC_TYPE_UINT64_T on Tru64 with gcc 3.4.4.

* lib/autoconf/types.m4 (_AC_TYPE_UNSIGNED_INT_BODY)
(_AC_TYPE_INT_BODY): Avoid undefined behavior of attempting shift
wider than type.
* NEWS: Document this.
Reported by Rainer Orth.

Signed-off-by: Eric Blake <ebb9@...>
---
 ChangeLog             |    9 +++++++++
 NEWS                  |    4 ++++
 lib/autoconf/types.m4 |   14 ++++++++++----
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 65b4a91..0470d59 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
[phone]  Eric Blake  <ebb9@...>
+
+ Fix AC_TYPE_UINT64_T on Tru64 with gcc 3.4.4.
+ * lib/autoconf/types.m4 (_AC_TYPE_UNSIGNED_INT_BODY)
+ (_AC_TYPE_INT_BODY): Avoid undefined behavior of attempting shift
+ wider than type.
+ * NEWS: Document this.
+ Reported by Rainer Orth.
+
 2009-09-16  Eric Blake  <ebb9@...>

  Optimize AC_REPLACE_FUNCS.
diff --git a/NEWS b/NEWS
index f738426..666cd4f 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ GNU Autoconf NEWS - User visible changes.
    made it fail with some Fortran compilers (regression introduced in
    2.64).

+** The AC_TYPE_UINT64_T and AC_TYPE_INT64_T macros have been fixed to no
+   longer mistakenly select a 32-bit type on some compilers (bug present
+   since macros were introduced in 2.59c).
+
 ** The following documented autotest macros are new:
    AT_CHECK_EUNIT

diff --git a/lib/autoconf/types.m4 b/lib/autoconf/types.m4
index a537967..7a73fc2 100644
--- a/lib/autoconf/types.m4
+++ b/lib/autoconf/types.m4
@@ -629,17 +629,21 @@ m4_define([_AC_TYPE_INT_BODY],
 [  AS_LINENO_PUSH([$[]1])
   AC_CACHE_CHECK([for int$[]2_t], [$[]3],
     [AS_VAR_SET([$[]3], [no])
+     # Order is important - never check a type that is potentially smaller
+     # than half of the expected target width.
      for ac_type in int$[]2_t 'int' 'long int' \
  'long long int' 'short int' 'signed char'; do
        AC_COMPILE_IFELSE(
  [AC_LANG_BOOL_COMPILE_TRY(
     [AC_INCLUDES_DEFAULT],
-    [0 < ($ac_type) (((($ac_type) 1 << ($[]2 - 2)) - 1) * 2 + 1)])],
+    [enum { N = $[]2 / 2 - 1 };
+     0 < ($ac_type) ((((($ac_type) 1 << N) << N) - 1) * 2 + 1)])],
  [AC_COMPILE_IFELSE(
     [AC_LANG_BOOL_COMPILE_TRY(
        [AC_INCLUDES_DEFAULT],
-       [($ac_type) (((($ac_type) 1 << ($[]2 - 2)) - 1) * 2 + 1)
- < ($ac_type) (((($ac_type) 1 << ($[]2 - 2)) - 1) * 2 + 2)])],
+       [enum { N = $[]2 / 2 - 1 };
+ ($ac_type) ((((($ac_type) 1 << N) << N) - 1) * 2 + 1)
+ < ($ac_type) ((((($ac_type) 1 << N) << N) - 1) * 2 + 2)])],
     [],
     [AS_CASE([$ac_type], [int$[]2_t],
        [AS_VAR_SET([$[]3], [yes])],
@@ -679,12 +683,14 @@ m4_define([_AC_TYPE_UNSIGNED_INT_BODY],
 [  AS_LINENO_PUSH([$[]1])
   AC_CACHE_CHECK([for uint$[]2_t], $[]3,
     [AS_VAR_SET([$[]3], [no])
+     # Order is important - never check a type that is potentially smaller
+     # than half of the expected target width.
      for ac_type in uint$[]2_t 'unsigned int' 'unsigned long int' \
  'unsigned long long int' 'unsigned short int' 'unsigned char'; do
        AC_COMPILE_IFELSE(
  [AC_LANG_BOOL_COMPILE_TRY(
     [AC_INCLUDES_DEFAULT],
-    [($ac_type) -1 >> ($[]2 - 1) == 1])],
+    [(($ac_type) -1 >> ($[]2 / 2 - 1)) >> ($[]2 / 2 - 1) == 3])],
  [AS_CASE([$ac_type], [uint$[]2_t],
     [AS_VAR_SET([$[]3], [yes])],
     [AS_VAR_SET([$[]3], [$ac_type])])])
--
1.6.4.2

[Paolo Bonzini-2]

On 10/20/2009 04:50 PM, Eric Blake wrote:
> An alternate patch would have been reordering the shell for loop to test long
> long, long, then int; that way, the checks are ordered widest to narrowest.
> But that has the potential for API changes if the macro selects a different
> type (but equivalent width) than what it did before the reordering, and it also
> penalizes the time taken for selecting the type to use for uint32_t.

ABI, actually, since it changes the C++ mangling.

Paolo

[Rainer Orth-2]

Eric Blake <ebb9@...> writes:

> Thanks for the report.  It is indeed a bug, and we should be able to work
> around it.  We need to break x>>63 into the more appropriate
> ((x>>31)>>31)>>1 to work around this.  But before I work on the necessary

Indeed: this works even with the old GCC.

> m4 magic, is it also a problem with smaller types?  In other words, what
> happens with:
>
> int
> main ()
> {
> static int test_array [1 - 2 * !((unsigned char) -1 >> (32 - 1) == 1)];
> test_array [0] = 0
>
>   ;
>   return 0;
> }

Even the old GCC 3.4.4 errors out with

conftest.c: In function `main':
conftest.c:4: error: size of array `test_array' is negative

Thanks.
        Rainer

--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University