Fix compilation errors with CC="clang -D_FORTIFY_SOURCE=2" on Android

Fix compilation errors with CC="clang -D_FORTIFY_SOURCE=2" on Android

[Bruno Haible]

[-- Attachment #1: Type: text/plain, Size: 8522 bytes --]

On Android, clang does not define _FORTIFY_SOURCE by default, but the Android
NDK sets CC to something like "clang -D_FORTIFY_SOURCE=2". Then, we see
compilation errors, such as

clang -ferror-limit=0 -D_FORTIFY_SOURCE=2 -DHAVE_CONFIG_H -DEXEEXT=\"\" -DEXEEXT=\"\" -DNO_XMALLOC -DEXEEXT=\"\" -I. -I../../gllib -I..  -DGNULIB_STRICT_CHECKING=1 -I/data/data/com.termux/files/home/local/include -Wall -fvisibility=hidden -O2 -c -o strverscmp.o ../../gllib/strverscmp.c
In file included from ../../gllib/strverscmp.c:26:
In file included from ./string.h:41:
In file included from /data/data/com.termux/files/usr/include/string.h:187:
/data/data/com.termux/files/usr/include/bits/fortify/string.h:95:47: error: use of undeclared identifier '__USE_FORTIFY_LEVEL'
        __clang_error_if(__bos_unevaluated_le(__bos(dst), __builtin_strlen(src)),
                                              ^
../../gllib/cdefs.h:144:48: note: expanded from macro '__bos'
#define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
                                               ^
In file included from ../../gllib/strverscmp.c:26:
In file included from ./string.h:41:
In file included from /data/data/com.termux/files/usr/include/string.h:187:
/data/data/com.termux/files/usr/include/bits/fortify/string.h:95:47: error: use of undeclared identifier '__USE_FORTIFY_LEVEL'
../../gllib/cdefs.h:144:48: note: expanded from macro '__bos'
#define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
                                               ^

Find the full list of errors from a Gnulib testdir attached.


The cause is that Android has its own framework for defining fortified
variants of the string operations, and like glibc it uses macros named
'__bos' and '__bos0'. But these macros have different definitions in
Android than in glibc. Using the glibc definition on Android leads to
trouble (because it references an undefined variable __USE_FORTIFY_LEVEL).
And undefining these macros after use leads to trouble as well (namely,
link errors for the undefined function '__bos0').

So, gnulib's cdefs.h and libc-config.h needs to avoid fiddling with
the Fortify stuff when used in Gnulib. The Gnulib modules that use
libc-config, namely
  canonicalize-lgpl
  fnmatch
  glibc-internal/dynarray
  glibc-internal/scratch_buffer
  glob, glob-h
  mktime
  nstrftime
  random, random_r
  strverscmp
  tempname
are unlikely to ever use this Fortify stuff.

This patch fixes the compilation errors. The use of __GNULIB_CDEFS has a
precedent; I therefore don't expect that it will trigger resistence during
the next merge back into glibc.


2023-01-29  Bruno Haible  <bruno@clisp.org>

	Fix compilation errors with CC="clang -D_FORTIFY_SOURCE=2" on Android.
	Reported by Alexey Rochev <equeim@gmail.com> in
	<https://lists.gnu.org/archive/html/bug-gnu-libiconv/2023-01/msg00019.html>.
	* lib/cdefs.h (__bos, __bos0, __glibc_objsize0, __glibc_objsize,
	__glibc_safe_len_cond, __glibc_unsigned_or_positive,
	__glibc_safe_or_unknown_len, __glibc_unsafe_len, __glibc_fortify,
	__glibc_fortify_n): Don't define these macros in Gnulib.
	* lib/libc-config.h: Don't undefine these macros in Gnulib.

diff --git a/lib/cdefs.h b/lib/cdefs.h
index 09a3d19b23..412f036ce3 100644
--- a/lib/cdefs.h
+++ b/lib/cdefs.h
@@ -140,32 +140,37 @@
 #endif
 
 
+/* Gnulib avoids these definitions, as they don't work on non-glibc platforms.
+   In particular, __bos and __bos0 are defined differently in the Android libc.
+ */
+#ifndef __GNULIB_CDEFS
+
 /* Fortify support.  */
-#define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
-#define __bos0(ptr) __builtin_object_size (ptr, 0)
+# define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
+# define __bos0(ptr) __builtin_object_size (ptr, 0)
 
 /* Use __builtin_dynamic_object_size at _FORTIFY_SOURCE=3 when available.  */
-#if __USE_FORTIFY_LEVEL == 3 && (__glibc_clang_prereq (9, 0)		      \
-				 || __GNUC_PREREQ (12, 0))
-# define __glibc_objsize0(__o) __builtin_dynamic_object_size (__o, 0)
-# define __glibc_objsize(__o) __builtin_dynamic_object_size (__o, 1)
-#else
-# define __glibc_objsize0(__o) __bos0 (__o)
-# define __glibc_objsize(__o) __bos (__o)
-#endif
+# if __USE_FORTIFY_LEVEL == 3 && (__glibc_clang_prereq (9, 0)		      \
+				  || __GNUC_PREREQ (12, 0))
+#  define __glibc_objsize0(__o) __builtin_dynamic_object_size (__o, 0)
+#  define __glibc_objsize(__o) __builtin_dynamic_object_size (__o, 1)
+# else
+#  define __glibc_objsize0(__o) __bos0 (__o)
+#  define __glibc_objsize(__o) __bos (__o)
+# endif
 
 /* Compile time conditions to choose between the regular, _chk and _chk_warn
    variants.  These conditions should get evaluated to constant and optimized
    away.  */
 
-#define __glibc_safe_len_cond(__l, __s, __osz) ((__l) <= (__osz) / (__s))
-#define __glibc_unsigned_or_positive(__l) \
+# define __glibc_safe_len_cond(__l, __s, __osz) ((__l) <= (__osz) / (__s))
+# define __glibc_unsigned_or_positive(__l) \
   ((__typeof (__l)) 0 < (__typeof (__l)) -1				      \
    || (__builtin_constant_p (__l) && (__l) > 0))
 
 /* Length is known to be safe at compile time if the __L * __S <= __OBJSZ
    condition can be folded to a constant and if it is true, or unknown (-1) */
-#define __glibc_safe_or_unknown_len(__l, __s, __osz) \
+# define __glibc_safe_or_unknown_len(__l, __s, __osz) \
   ((__osz) == (__SIZE_TYPE__) -1					      \
    || (__glibc_unsigned_or_positive (__l)				      \
        && __builtin_constant_p (__glibc_safe_len_cond ((__SIZE_TYPE__) (__l), \
@@ -175,7 +180,7 @@
 /* Conversely, we know at compile time that the length is unsafe if the
    __L * __S <= __OBJSZ condition can be folded to a constant and if it is
    false.  */
-#define __glibc_unsafe_len(__l, __s, __osz) \
+# define __glibc_unsafe_len(__l, __s, __osz) \
   (__glibc_unsigned_or_positive (__l)					      \
    && __builtin_constant_p (__glibc_safe_len_cond ((__SIZE_TYPE__) (__l),     \
 						   __s, __osz))		      \
@@ -184,7 +189,7 @@
 /* Fortify function f.  __f_alias, __f_chk and __f_chk_warn must be
    declared.  */
 
-#define __glibc_fortify(f, __l, __s, __osz, ...) \
+# define __glibc_fortify(f, __l, __s, __osz, ...) \
   (__glibc_safe_or_unknown_len (__l, __s, __osz)			      \
    ? __ ## f ## _alias (__VA_ARGS__)					      \
    : (__glibc_unsafe_len (__l, __s, __osz)				      \
@@ -194,13 +199,16 @@
 /* Fortify function f, where object size argument passed to f is the number of
    elements and not total size.  */
 
-#define __glibc_fortify_n(f, __l, __s, __osz, ...) \
+# define __glibc_fortify_n(f, __l, __s, __osz, ...) \
   (__glibc_safe_or_unknown_len (__l, __s, __osz)			      \
    ? __ ## f ## _alias (__VA_ARGS__)					      \
    : (__glibc_unsafe_len (__l, __s, __osz)				      \
       ? __ ## f ## _chk_warn (__VA_ARGS__, (__osz) / (__s))		      \
       : __ ## f ## _chk (__VA_ARGS__, (__osz) / (__s))))		      \
 
+#endif
+
+
 #if __GNUC_PREREQ (4,3)
 # define __warnattr(msg) __attribute__((__warning__ (msg)))
 # define __errordecl(name, msg) \
diff --git a/lib/libc-config.h b/lib/libc-config.h
index 1d28e58c97..5f5ad01037 100644
--- a/lib/libc-config.h
+++ b/lib/libc-config.h
@@ -137,8 +137,6 @@
 # undef __attribute_returns_twice__
 # undef __attribute_used__
 # undef __attribute_warn_unused_result__
-# undef __bos
-# undef __bos0
 # undef __errordecl
 # undef __extension__
 # undef __extern_always_inline
@@ -147,21 +145,13 @@
 # undef __fortified_attr_access
 # undef __fortify_function
 # undef __glibc_c99_flexarr_available
-# undef __glibc_fortify
-# undef __glibc_fortify_n
 # undef __glibc_has_attribute
 # undef __glibc_has_builtin
 # undef __glibc_has_extension
 # undef __glibc_likely
 # undef __glibc_macro_warning
 # undef __glibc_macro_warning1
-# undef __glibc_objsize
-# undef __glibc_objsize0
-# undef __glibc_safe_len_cond
-# undef __glibc_safe_or_unknown_len
 # undef __glibc_unlikely
-# undef __glibc_unsafe_len
-# undef __glibc_unsigned_or_positive
 # undef __inline
 # undef __ptr_t
 # undef __restrict
@@ -170,6 +160,18 @@
 # undef __va_arg_pack_len
 # undef __warnattr
 # undef __wur
+# ifndef __GNULIB_CDEFS
+#  undef __bos
+#  undef __bos0
+#  undef __glibc_fortify
+#  undef __glibc_fortify_n
+#  undef __glibc_objsize
+#  undef __glibc_objsize0
+#  undef __glibc_safe_len_cond
+#  undef __glibc_safe_or_unknown_len
+#  undef __glibc_unsafe_len
+#  undef __glibc_unsigned_or_positive
+# endif
 
 /* Include our copy of glibc <sys/cdefs.h>.  */
 # include <cdefs.h>

[-- Attachment #2: android-errors.log.gz --]
[-- Type: application/gzip, Size: 15742 bytes --]

libc-config and system-reserved symbols

[Bruno Haible]

Hi Paul,

I wrote:
> The cause is that Android has its own framework for defining fortified
> variants of the string operations, and like glibc it uses macros named
> '__bos' and '__bos0'. But these macros have different definitions in
> Android than in glibc.

This situation can happen with any symbols that are prefixed with __
and are therefore system-reserved.

As far as I understand, the general approach of this cdefs.h is that
it defines __ prefixed macros for glibc, and there are two categories:
  (A) Those that become active in gnulib as well.
  (B) Those that are not active in gnulib.

For porting more code from glibc to gnulib, and for avoiding surprises
when pulling new versions of existing modules from glibc to gnulib,
it is useful when the set (A) is large.

On the other hand, to avoid conflicts with existing platforms, the
set (A) should be small.

Today, I moved the Fortify-related symbols from (A) to (B).

But I claim that the set (A) is still too large.

1) Look at the defined symbols: Some, such as __P, __PMT, etc.
   are not used by the code that Gnulib borrows from glibc.
   Each such symbol could have a meaning different from the glibc one
   on some platform.

2) Speaking specifically about Android, I compared the definitions.
After eliminating equivalentr definitions, the remainder is:

Gnulib <cdefs.h>:
#  define __inline      inline
#  define __inline              /* No inline functions.  */
# define __THROW
# define __THROWNL
# define __NTH(fct)     fct
#define __CONCAT(x,y)   x ## y
#define __STRING(x)     #x
#define __bos(ptr) __builtin_object_size (ptr, __USE_FORTIFY_LEVEL > 1)
#define __bos0(ptr) __builtin_object_size (ptr, 0)
# define __warnattr(msg) __attribute__((__warning__ (msg)))
# define __warnattr(msg)
# define __attribute_warn_unused_result__ \
#  define __wur __attribute_warn_unused_result__
# define __attribute_warn_unused_result__ /* empty */
# define __always_inline __inline __attribute__ ((__always_inline__))
# define __always_inline __inline
#  define __extern_inline extern __inline __attribute__ ((__gnu_inline__))
#  define __extern_always_inline \
#  define __extern_inline extern __inline
#  define __extern_always_inline extern __always_inline

Android <sys/cdefs.h>:
#define __CONCAT1(x,y)  x ## y
#define __CONCAT(x,y)   __CONCAT1(x,y)
#define ___CONCAT(x,y)  __CONCAT(x,y)
#define __inline        inline          /* convert to C++ keyword */
#define __always_inline __attribute__((__always_inline__))
#define __warnattr(msg) __attribute__((deprecated(msg)))
#define __bos(s) __bosn((s), __bos_level)
#  define __bos0(s) __bosn((s), 0)

Notice in particular:
  - the different definitions of __CONCAT
  - the different definitions of __inline
  - the different definitions of __always_inline
  - the different definitions of __warnattr

I would not be surprised if these macros cause trouble, either on Android
or on other platforms, in the future.

Bruno

Re: libc-config and system-reserved symbols

[Paul Eggert]

On 2023-01-29 15:20, Bruno Haible wrote:
> Notice in particular:
>    - the different definitions of __CONCAT
>    - the different definitions of __inline
>    - the different definitions of __always_inline
>    - the different definitions of __warnattr
> 
> I would not be surprised if these macros cause trouble, either on Android
> or on other platforms, in the future.

Yes, we are on thin ice here. If there is trouble I suppose we'll need 
to conditionally define and use _GL_CONCAT etc. to work around it. Hope 
it's not needed.