diff mbox

Correct type of SSIZE_MAX for 32-bit (bug 13575)

Message ID alpine.DEB.2.20.1802012036530.13065@digraph.polyomino.org.uk
State New
Headers show

Commit Message

Joseph Myers Feb. 1, 2018, 8:37 p.m. UTC
Bug 13575 reports that SSIZE_MAX is wrongly defined as LONG_MAX on
32-bit systems where ssize_t is defined as int (which is most 32-bit
systems supported by glibc).

This patch fixes the definition, using a conditional on
__WORDSIZE32_SIZE_ULONG to determine the appropriate type in the
32-bit case.  Formally ssize_t need not be the signed type
corresponding to size_t, but as it is for all current glibc
configurations, there is no need for a new macro different from the
one used for defining SIZE_MAX.  A testcase is added for both the type
and the value of SSIZE_MAX.

There is a relevant peculiarity in
sysdeps/unix/sysv/linux/s390/bits/typesizes.h:

#if defined __GNUC__ && __GNUC__ <= 2
/* Compatibility with g++ 2.95.x.  */
#define __SSIZE_T_TYPE          __SWORD_TYPE
#else
/* size_t is unsigned long int on s390 -m31.  */
#define __SSIZE_T_TYPE          __SLONGWORD_TYPE
#endif

This has the effect that for GCC 2 for s390, ssize_t does not match
__WORDSIZE32_SIZE_ULONG.  I don't think such a conditional on the GCC
version makes sense - to have a well-defined ABI, the choices of
standard types should not depend on the GCC version.  It's also the
case that upstream GCC 2.95 did not support s390, and glibc headers
don't in general try to support past development GCC versions - only
actual releases and current mainline development.  But whether or not
that GCC 2 case should be removed (with or without a NEWS entry for
such a change), this patch does not result in any changes for s390;
the value is always still LONG_MAX in the s390 case because
__WORDSIZE32_SIZE_ULONG is always defined for 32-bit s390.  I don't
think any such oddity in code only active for unofficial or unreleased
old compiler versions should block closing the present bug as fixed
once this patch is in.

Tested for x86_64 and x86, and with build-many-glibcs.py.

2018-02-01  Joseph Myers  <joseph@codesourcery.com>

	[BZ #13575]
	* posix/bits/posix1_lim.h: Include <bits/wordsize.h>.
	[!SSIZE_MAX && !(__WORDSIZE == 64 || __WORDSIZE32_SIZE_ULONG)]
	(SSIZE_MAX): Define to INT_MAX.
	* posix/test-ssize-max.c: New file.
	* posix/Makefile (tests): Add test-ssize-max.

Comments

Joseph Myers Feb. 6, 2018, 6:02 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/ml/libc-alpha/2018-02/msg00035.html> is pending 
review.
Florian Weimer Feb. 6, 2018, 6:38 p.m. UTC | #2
On 02/01/2018 09:37 PM, Joseph Myers wrote:
> +ssize_t x;
> +extern __typeof (SSIZE_MAX) x;

Does this assert the equivalence of the types (just like a pointer 
assignment would do)?  Maybe at a comment to that effect?

I'm not sure if __WORDSIZE32_SIZE_ULONG is the right condition for that 
because it does not seem to govern the actual definition if 
size_t/ssize_t, but with the compile test to check for consistency, it's 
good enough, I think.

Thanks,
Florian
Joseph Myers Feb. 6, 2018, 9:41 p.m. UTC | #3
On Tue, 6 Feb 2018, Florian Weimer wrote:

> On 02/01/2018 09:37 PM, Joseph Myers wrote:
> > +ssize_t x;
> > +extern __typeof (SSIZE_MAX) x;
> 
> Does this assert the equivalence of the types (just like a pointer assignment
> would do)?  Maybe at a comment to that effect?

Yes, this asserts equivalence of types (without any dependence on -Werror, 
incompatible types for the same object produces an unconditional hard 
error).  I've added a comment "Test SSIZE_MAX has type ssize_t.".

> I'm not sure if __WORDSIZE32_SIZE_ULONG is the right condition for that
> because it does not seem to govern the actual definition if size_t/ssize_t,
> but with the compile test to check for consistency, it's good enough, I think.

I think this is already adequately explained in both the patch description 
and the comment

/* ssize_t is not formally required to be the signed type
   corresponding to size_t, but it is for all configurations supported
   by glibc.  */

on the definition of SSIZE_MAX.
diff mbox

Patch

diff --git a/posix/Makefile b/posix/Makefile
index 83b3d74..51dcf12 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -95,7 +95,7 @@  tests		:= test-errno tstgetopt testfnm runtests runptests \
 		   tst-posix_spawn-fd tst-posix_spawn-setsid \
 		   tst-posix_fadvise tst-posix_fadvise64 \
 		   tst-sysconf-empty-chroot tst-glob_symlinks tst-fexecve \
-		   tst-glob-tilde
+		   tst-glob-tilde test-ssize-max
 tests-internal	:= bug-regex5 bug-regex20 bug-regex33 \
 		   tst-rfc3484 tst-rfc3484-2 tst-rfc3484-3 \
 		   tst-glob_lstat_compat
diff --git a/posix/bits/posix1_lim.h b/posix/bits/posix1_lim.h
index 90066ff..18ac47f 100644
--- a/posix/bits/posix1_lim.h
+++ b/posix/bits/posix1_lim.h
@@ -24,6 +24,7 @@ 
 #ifndef	_BITS_POSIX1_LIM_H
 #define	_BITS_POSIX1_LIM_H	1
 
+#include <bits/wordsize.h>
 
 /* These are the standard-mandated minimum values.  */
 
@@ -161,7 +162,14 @@ 
 
 
 #ifndef	SSIZE_MAX
-# define SSIZE_MAX	LONG_MAX
+/* ssize_t is not formally required to be the signed type
+   corresponding to size_t, but it is for all configurations supported
+   by glibc.  */
+# if __WORDSIZE == 64 || __WORDSIZE32_SIZE_ULONG
+#  define SSIZE_MAX	LONG_MAX
+# else
+#  define SSIZE_MAX	INT_MAX
+# endif
 #endif
 
 
diff --git a/posix/test-ssize-max.c b/posix/test-ssize-max.c
new file mode 100644
index 0000000..12e3d8e
--- /dev/null
+++ b/posix/test-ssize-max.c
@@ -0,0 +1,36 @@ 
+/* Test SSIZE_MAX value and type.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <limits.h>
+#include <sys/types.h>
+
+ssize_t x;
+extern __typeof (SSIZE_MAX) x;
+_Static_assert (SSIZE_MAX == (sizeof (ssize_t) == sizeof (int)
+			      ? INT_MAX
+			      : LONG_MAX),
+		"value of SSIZE_MAX");
+
+static int
+do_test (void)
+{
+  /* This is a compilation test.  */
+  return 0;
+}
+
+#include <support/test-driver.c>