diff mbox

[v2,BZ,#17979,BZ,#17721] Fix issues with sys/cdefs.h and uchar.h when using non-gcc compiler.

Message ID 14a0632d481a4be80811e7f1e0675432.squirrel@server316.webhostingpad.com
State New, archived
Headers show

Commit Message

Dwight Guth Feb. 1, 2016, 9:44 a.m. UTC
If linking against Glibc with a compiler for which the __GNUC__ macro is not
defined, problems arise when including header files that use the __restrict
or __inline keyword and when including uchar.h.

Glibc strips __restrict from the prototypes of C library functions in this
case. This is undesirable if the compiler is a C99-compliant compiler,
because
C99 includes the restrict keyword and uses it in the declaration of a number
of functions in the C library. While this is broadly correct, lack of this
information can have undesirable effects on analysis tools. The same thing
occurs with the __inline keyword, which is also defined in C99 but stripped
if the compiler is not GNU C. Here we except the case where the compiler
declares itself to be C99-compliant from these checks in order to allow
better
C99 compliance for non-GNU-C compilers which link against Glibc.

Glibc defines char16_t and char32_t in uchar.h as __CHAR16_TYPE__ and
__CHAR32_TYPE__ when the __GNUC__ macro is defined, but when linking against
Glibc with a different compiler, these types are not defined at all,
which is a violation of C11 sec. 7.28 paragraph 2, as well as a syntax
error because these types are used in the prototypes of functions declared
later in the file. According to this section of the standard, these types
must be defined in this header file and must be the same type as
uint_least16_t and uint_least32_t, which are defined in stdint.h as
"unsigned short int" and "unsigned int" respectively. Here we modify the
header so that if __GNUC__ is not defined, we still provide these typedefs,
but we obtain them from bits/stdint.h, where they are defined to be equal
to uint_least16_t and uint_least32_t, if __CHAR16_TYPE__ and __CHAR32_TYPE__
are not defined by the compiler.

---

I had trouble testing this patch because I ran into unrelated errors in the
test suite. No tests seem to fail on my machine after the patch that were
passing before, however. It seems to happen because it can't dynamically
link against libgcc_s, but I'm not 100% sure what the correct way to pass
that
to make check is. Can someone point me to where this documentation is?

---

Changed from previous version:

* Updated patch description based on comments on mailing list.
* Added a bits/stdint.h which defines __uint_least16_t and __uint_least32_t
  and which is included from stdint.h and uchar.h.

I didn't move stdint.h out of sysdeps because I wasn't sure how controversial
that would be, but I can do that in a third version of the patch if need be.
I was under impression that the issue was that uchar.h was outside sysdeps
and could not directly assume a particular version of stdint.h, but that
assuming that whatever particular version of bits/stdint.h you had would
define
a certain type, and including that header from outside sysdeps should be
fine.
If this is not the case, apologies, just let me know.

---

2016-01-28  Dwight Guth  <dwight.guth@runtimeverification.com>

	[BZ #17979]
	* wcsmbs/uchar.h (char16_t, char32_t): Define types if __GNUC__,
	__CHAR16_TYPE__, or __CHAR32_TYPE__ are not defined.
	* sysdeps/generic/bits/stdint.h: Created.
	* sysdeps/generic/stdint.h (uint_least16_t, uint_least32_t): Defined
	based on types from bits/stdint.h.

	[BZ #17721]
	* misc/sys/cdefs.h (__restrict, __inline): Define as keywords if
	__GNUC__ is not defined but __STDC_VERSION__ is at least C99.

---

Comments

Dwight Guth Feb. 10, 2016, 4:16 a.m. UTC | #1
Pinging this because I didn't get a response.

On Mon, February 1, 2016 3:44 am, Dwight Guth wrote:
> If linking against Glibc with a compiler for which the __GNUC__ macro is
> not defined, problems arise when including header files that use the
> __restrict
> or __inline keyword and when including uchar.h.
>
> Glibc strips __restrict from the prototypes of C library functions in
> this case. This is undesirable if the compiler is a C99-compliant
> compiler, because C99 includes the restrict keyword and uses it in the
> declaration of a number of functions in the C library. While this is
> broadly correct, lack of this information can have undesirable effects on
> analysis tools. The same thing occurs with the __inline keyword, which is
> also defined in C99 but stripped if the compiler is not GNU C. Here we
> except the case where the compiler declares itself to be C99-compliant
> from these checks in order to allow better C99 compliance for non-GNU-C
> compilers which link against Glibc.
>
> Glibc defines char16_t and char32_t in uchar.h as __CHAR16_TYPE__ and
> __CHAR32_TYPE__ when the __GNUC__ macro is defined, but when linking
> against Glibc with a different compiler, these types are not defined at
> all, which is a violation of C11 sec. 7.28 paragraph 2, as well as a
> syntax error because these types are used in the prototypes of functions
> declared later in the file. According to this section of the standard,
> these types must be defined in this header file and must be the same type
> as uint_least16_t and uint_least32_t, which are defined in stdint.h as
> "unsigned short int" and "unsigned int" respectively. Here we modify the
> header so that if __GNUC__ is not defined, we still provide these
> typedefs, but we obtain them from bits/stdint.h, where they are defined to
> be equal to uint_least16_t and uint_least32_t, if __CHAR16_TYPE__ and
> __CHAR32_TYPE__
> are not defined by the compiler.
>
> ---
>
>
> I had trouble testing this patch because I ran into unrelated errors in
> the test suite. No tests seem to fail on my machine after the patch that
> were passing before, however. It seems to happen because it can't
> dynamically link against libgcc_s, but I'm not 100% sure what the correct
> way to pass that to make check is. Can someone point me to where this
> documentation is?
>
> ---
>
>
> Changed from previous version:
>
>
> * Updated patch description based on comments on mailing list.
> * Added a bits/stdint.h which defines __uint_least16_t and
> __uint_least32_t
> and which is included from stdint.h and uchar.h.
>
> I didn't move stdint.h out of sysdeps because I wasn't sure how
> controversial that would be, but I can do that in a third version of the
> patch if need be. I was under impression that the issue was that uchar.h
> was outside sysdeps and could not directly assume a particular version of
> stdint.h, but that assuming that whatever particular version of
> bits/stdint.h you had would define a certain type, and including that
> header from outside sysdeps should be fine. If this is not the case,
> apologies, just let me know.
>
> ---
>
>
> 2016-01-28  Dwight Guth  <dwight.guth@runtimeverification.com>
>
>
> [BZ #17979]
> * wcsmbs/uchar.h (char16_t, char32_t): Define types if __GNUC__,
> __CHAR16_TYPE__, or __CHAR32_TYPE__ are not defined.
> * sysdeps/generic/bits/stdint.h: Created.
> * sysdeps/generic/stdint.h (uint_least16_t, uint_least32_t): Defined
> based on types from bits/stdint.h.
>
> [BZ #17721]
> * misc/sys/cdefs.h (__restrict, __inline): Define as keywords if
> __GNUC__ is not defined but __STDC_VERSION__ is at least C99.
>
>
> ---
>
>
> diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 7fd4154..af23ff7
> 100644
> --- a/misc/sys/cdefs.h
> +++ b/misc/sys/cdefs.h
> @@ -69,8 +69,11 @@
>
>
> #else	/* Not GCC.  */
>
>
> -# define __inline		/* No inline functions.  */
> -
> +# if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
> +#  define __inline		inline
> +# else
> +#  define __inline		/* No inline functions.  */
> +# endif
> # define __THROW
> # define __THROWNL
> # define __NTH(fct)	fct
> @@ -360,7 +363,11 @@
>
>
> /* __restrict is known in EGCS 1.2 and above. */
> #if !__GNUC_PREREQ (2,92)
> -# define __restrict	/* Ignore */
> +# if !defined __GNUC__ && defined __STDC_VERSION__ && __STDC_VERSION__ >=
>  199901L
> +#  define __restrict	restrict
> +# else
> +#  define __restrict	/* Ignore */
> +# endif
> #endif
>
>
> /* ISO C99 also allows to declare arrays as non-overlapping.  The syntax
> is diff --git a/sysdeps/generic/bits/stdint.h
> b/sysdeps/generic/bits/stdint.h new file mode 100644 index 0000000..ee63a2a
>  --- /dev/null
> +++ b/sysdeps/generic/bits/stdint.h
> @@ -0,0 +1,29 @@
> +/* uint_least16_t and uint_least32_t definitions
> +   Copyright (C) 2016 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/>.  */
> +
> +#ifndef _BITS_STDINT_H
> +#define _BITS_STDINT_H	1
> +
> +/* uint_least16_t and char16_t must be the same type, as must
> +   uint_least32_t and char32_t. We define here a reserved identifier
> +   for these types so that they will always remain the same type. */
> +
> +typedef unsigned short int	__uint_least16_t;
> +typedef unsigned int		__uint_least32_t;
> +
> +#endif	/* bits/stdint.h */
> diff --git a/sysdeps/generic/stdint.h b/sysdeps/generic/stdint.h index
> 4427627..2f4ebd2 100644
> --- a/sysdeps/generic/stdint.h
> +++ b/sysdeps/generic/stdint.h
> @@ -25,6 +25,7 @@
> #include <features.h>
> #include <bits/wchar.h>
> #include <bits/wordsize.h>
> +#include <bits/stdint.h>
>
>
> /* Exact integral types.  */
>
>
> @@ -74,8 +75,8 @@ typedef long long int		int_least64_t;
>
>
> /* Unsigned.  */
> typedef unsigned char		uint_least8_t; -typedef unsigned short int
> uint_least16_t; -typedef unsigned int		uint_least32_t;
> +typedef __uint_least16_t	uint_least16_t;
> +typedef __uint_least32_t	uint_least32_t;
> #if __WORDSIZE == 64
> typedef unsigned long int	uint_least64_t; #else
> diff --git a/wcsmbs/uchar.h b/wcsmbs/uchar.h index ce92b25..875919a 100644
> --- a/wcsmbs/uchar.h
> +++ b/wcsmbs/uchar.h
> @@ -30,6 +30,8 @@
> #define __need_mbstate_t
> #include <wchar.h>
>
>
> +#include <bits/stdint.h>
> +
> #ifndef __mbstate_t_defined
> __BEGIN_NAMESPACE_C99
> /* Public type.  */
> @@ -39,14 +41,16 @@ __END_NAMESPACE_C99
> #endif
>
>
>
> -#if defined __GNUC__ && !defined __USE_ISOCXX11
> +#if !defined __USE_ISOCXX11
> /* Define the 16-bit and 32-bit character types.  Use the information
> provided by the compiler.  */ # if !defined __CHAR16_TYPE__ || !defined
> __CHAR32_TYPE__
> #  if defined __STDC_VERSION__ && __STDC_VERSION__ < 201000L
> #   error "<uchar.h> requires ISO C11 mode"
> #  else
> -#   error "definitions of __CHAR16_TYPE__ and/or __CHAR32_TYPE__ missing"
>  +/* Same as uint_least16_t and uint_least32_t in stdint.h. */
> +typedef __uint_least16_t __CHAR16_TYPE__;
> +typedef __uint_least32_t __CHAR32_TYPE__;
> #  endif
> # endif
> typedef __CHAR16_TYPE__ char16_t;
>
Joseph Myers Feb. 10, 2016, 12:59 p.m. UTC | #2
On Tue, 9 Feb 2016, Dwight Guth wrote:

> Pinging this because I didn't get a response.

We're still in freeze for 2.23.  I advise resubmitting, split into 
separate patches for each issue, once 2.23 is out.
Mike Frysinger Feb. 10, 2016, 5:21 p.m. UTC | #3
i think this should be split up into two patches ... one for restrict
and one for types.  when master re-opens, feel free to ping then.
-mike
diff mbox

Patch

diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index 7fd4154..af23ff7 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -69,8 +69,11 @@ 

 #else	/* Not GCC.  */

-# define __inline		/* No inline functions.  */
-
+# if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
+#  define __inline		inline
+# else
+#  define __inline		/* No inline functions.  */
+# endif
 # define __THROW
 # define __THROWNL
 # define __NTH(fct)	fct
@@ -360,7 +363,11 @@ 

 /* __restrict is known in EGCS 1.2 and above. */
 #if !__GNUC_PREREQ (2,92)
-# define __restrict	/* Ignore */
+# if !defined __GNUC__ && defined __STDC_VERSION__ && __STDC_VERSION__ >=
199901L
+#  define __restrict	restrict
+# else
+#  define __restrict	/* Ignore */
+# endif
 #endif

 /* ISO C99 also allows to declare arrays as non-overlapping.  The syntax is
diff --git a/sysdeps/generic/bits/stdint.h b/sysdeps/generic/bits/stdint.h
new file mode 100644
index 0000000..ee63a2a
--- /dev/null
+++ b/sysdeps/generic/bits/stdint.h
@@ -0,0 +1,29 @@ 
+/* uint_least16_t and uint_least32_t definitions
+   Copyright (C) 2016 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/>.  */
+
+#ifndef _BITS_STDINT_H
+#define _BITS_STDINT_H	1
+
+/* uint_least16_t and char16_t must be the same type, as must
+   uint_least32_t and char32_t. We define here a reserved identifier
+   for these types so that they will always remain the same type. */
+
+typedef unsigned short int	__uint_least16_t;
+typedef unsigned int		__uint_least32_t;
+
+#endif	/* bits/stdint.h */
diff --git a/sysdeps/generic/stdint.h b/sysdeps/generic/stdint.h
index 4427627..2f4ebd2 100644
--- a/sysdeps/generic/stdint.h
+++ b/sysdeps/generic/stdint.h
@@ -25,6 +25,7 @@ 
 #include <features.h>
 #include <bits/wchar.h>
 #include <bits/wordsize.h>
+#include <bits/stdint.h>

 /* Exact integral types.  */

@@ -74,8 +75,8 @@  typedef long long int		int_least64_t;

 /* Unsigned.  */
 typedef unsigned char		uint_least8_t;
-typedef unsigned short int	uint_least16_t;
-typedef unsigned int		uint_least32_t;
+typedef __uint_least16_t	uint_least16_t;
+typedef __uint_least32_t	uint_least32_t;
 #if __WORDSIZE == 64
 typedef unsigned long int	uint_least64_t;
 #else
diff --git a/wcsmbs/uchar.h b/wcsmbs/uchar.h
index ce92b25..875919a 100644
--- a/wcsmbs/uchar.h
+++ b/wcsmbs/uchar.h
@@ -30,6 +30,8 @@ 
 #define __need_mbstate_t
 #include <wchar.h>

+#include <bits/stdint.h>
+
 #ifndef __mbstate_t_defined
 __BEGIN_NAMESPACE_C99
 /* Public type.  */
@@ -39,14 +41,16 @@  __END_NAMESPACE_C99
 #endif


-#if defined __GNUC__ && !defined __USE_ISOCXX11
+#if !defined __USE_ISOCXX11
 /* Define the 16-bit and 32-bit character types.  Use the information
    provided by the compiler.  */
 # if !defined __CHAR16_TYPE__ || !defined __CHAR32_TYPE__
 #  if defined __STDC_VERSION__ && __STDC_VERSION__ < 201000L
 #   error "<uchar.h> requires ISO C11 mode"
 #  else
-#   error "definitions of __CHAR16_TYPE__ and/or __CHAR32_TYPE__ missing"
+/* Same as uint_least16_t and uint_least32_t in stdint.h. */
+typedef __uint_least16_t __CHAR16_TYPE__;
+typedef __uint_least32_t __CHAR32_TYPE__;
 #  endif
 # endif
 typedef __CHAR16_TYPE__ char16_t;