Subject: Re: 24.5; Build failure with modules on OpenBSD
From: Paul Eggert <[email-masked]>
To: Joakim Jalap <[email-masked]>
Cc: Gabriele Svelto <[email-masked]>, [email-masked]
Date: Sun, 17 Jan 2016 14:30:46 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for reporting the problem. The OpenBSD compiler warnings about strcpy 
etc. are false alarms, and these false alarms confuse the Emacs build procedure 
and therefore introduce bugs into the Emacs module code. We're lucky that a 
run-time assertion caught this.

I installed the attached patch into the emacs-25 branch. It removes the 
offending test in 'configure', and so should fix the bug so I am boldly marking 
it as done. Please give the fix a try; if it doesn't work I can unmark the bug.

Is there some way to tell the OpenBSD C compiler to not chatter about uses of 
strcpy, strcat, and sprintf? We don't want to use the OpenBSD-suggested 
replacements in Emacs, as that would be more likely to introduce bugs than to 
fix them. If we can suggest (or get 'configure' to use) a build-time option to 
suppress the false alarms that would save everybody some work and would help 
make Emacs more reliable.

Although the OpenBSD porting problem with AX_GCC_VAR_ATTRIBUTE no longer affects 
Emacs, it might affect other programs that use that macro, so I'll CC: this to 
the macro's author Gabriele Svelto. Gabriele, the bug report is here:

http://bugs.gnu.org/22385
From 1addf499c8318ecf2b00e1c60dfa56e67bc1689e Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 17 Jan 2016 14:10:26 -0800
Subject: [PATCH] Port cleanup attribute to OpenBSD
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The OpenBSD C compiler issues false alarms about strcpy, strcat, and
sprintf, and this messes up 'configure' when it tests for the cleanup
attribute.  Work around the problem by using __has_attribute directly.
Problem reported by Joakim Jalap (Bug#22385).
* configure.ac: Don’t use AX_GCC_VAR_ATTRIBUTE.
* m4/ax_gcc_var_attribute.m4: Remove.
* src/conf_post.h (__has_attribute): Provide a substitute, for
non-GCC or older GCC compilers.  All uses changed to assume
the substitute.  Check for the cleanup attribute.
* src/emacs-module.c (module_has_cleanup): Just use __has_attribute.
---
 configure.ac               |   1 -
 m4/ax_gcc_var_attribute.m4 | 141 ---------------------------------------------
 src/conf_post.h            |  28 +++++----
 src/emacs-module.c         |   3 +-
 4 files changed, 18 insertions(+), 155 deletions(-)
 delete mode 100644 m4/ax_gcc_var_attribute.m4

diff --git a/configure.ac b/configure.ac
index 8c01aba..3aeba22 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3327,7 +3327,6 @@ if test "${HAVE_MODULES}" = yes; then
 fi
 AC_SUBST(MODULES_OBJ)
 AC_SUBST(LIBMODULES)
-AX_GCC_VAR_ATTRIBUTE(cleanup)
 AC_CHECK_FUNCS(dladdr)
 
 ### Use -lpng if available, unless '--with-png=no'.
diff --git a/m4/ax_gcc_var_attribute.m4 b/m4/ax_gcc_var_attribute.m4
deleted file mode 100644
index d12fce8..0000000
--- a/m4/ax_gcc_var_attribute.m4
+++ /dev/null
@@ -1,141 +0,0 @@
-# ===========================================================================
-#   http://www.gnu.org/software/autoconf-archive/ax_gcc_var_attribute.html
-# ===========================================================================
-#
-# SYNOPSIS
-#
-#   AX_GCC_VAR_ATTRIBUTE(ATTRIBUTE)
-#
-# DESCRIPTION
-#
-#   This macro checks if the compiler supports one of GCC's variable
-#   attributes; many other compilers also provide variable attributes with
-#   the same syntax. Compiler warnings are used to detect supported
-#   attributes as unsupported ones are ignored by default so quieting
-#   warnings when using this macro will yield false positives.
-#
-#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
-#
-#   If ATTRIBUTE is supported define HAVE_VAR_ATTRIBUTE_<ATTRIBUTE>.
-#
-#   The macro caches its result in the ax_cv_have_var_attribute_<attribute>
-#   variable.
-#
-#   The macro currently supports the following variable attributes:
-#
-#    aligned
-#    cleanup
-#    common
-#    nocommon
-#    deprecated
-#    mode
-#    packed
-#    tls_model
-#    unused
-#    used
-#    vector_size
-#    weak
-#    dllimport
-#    dllexport
-#    init_priority
-#
-#   Unsupported variable attributes will be tested against a global integer
-#   variable and without any arguments given to the attribute itself; the
-#   result of this check might be wrong or meaningless so use with care.
-#
-# LICENSE
-#
-#   Copyright (c) 2013 Gabriele Svelto <[email-masked]>
-#
-#   Copying and distribution of this file, with or without modification, are
-#   permitted in any medium without royalty provided the copyright notice
-#   and this notice are preserved.  This file is offered as-is, without any
-#   warranty.
-
-#serial 3
-
-AC_DEFUN([AX_GCC_VAR_ATTRIBUTE], [
-    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_var_attribute_$1])
-
-    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
-        AC_LINK_IFELSE([AC_LANG_PROGRAM([
-            m4_case([$1],
-                [aligned], [
-                    int foo __attribute__(($1(32)));
-                ],
-                [cleanup], [
-                    int bar(int *t) { return *t; };
-                ],
-                [common], [
-                    int foo __attribute__(($1));
-                ],
-                [nocommon], [
-                    int foo __attribute__(($1));
-                ],
-                [deprecated], [
-                    int foo __attribute__(($1)) = 0;
-                ],
-                [mode], [
-                    long foo __attribute__(($1(word)));
-                ],
-                [packed], [
-                    struct bar {
-                        int baz __attribute__(($1));
-                    };
-                ],
-                [tls_model], [
-                    __thread int bar1 __attribute__(($1("global-dynamic")));
-                    __thread int bar2 __attribute__(($1("local-dynamic")));
-                    __thread int bar3 __attribute__(($1("initial-exec")));
-                    __thread int bar4 __attribute__(($1("local-exec")));
-                ],
-                [unused], [
-                    int foo __attribute__(($1));
-                ],
-                [used], [
-                    int foo __attribute__(($1));
-                ],
-                [vector_size], [
-                    int foo __attribute__(($1(16)));
-                ],
-                [weak], [
-                    int foo __attribute__(($1));
-                ],
-                [dllimport], [
-                    int foo __attribute__(($1));
-                ],
-                [dllexport], [
-                    int foo __attribute__(($1));
-                ],
-                [init_priority], [
-                    struct bar { bar() {} ~bar() {} };
-                    bar b __attribute__(($1(65535/2)));
-                ],
-                [
-                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
-                 int foo __attribute__(($1));
-                ]
-            )], [
-            m4_case([$1],
-                [cleanup], [
-                    int foo __attribute__(($1(bar))) = 0;
-                    foo = foo + 1;
-                ],
-                []
-            )])
-            ],
-            dnl GCC doesn't exit with an error if an unknown attribute is
-            dnl provided but only outputs a warning, so accept the attribute
-            dnl only if no warning were issued.
-            [AS_IF([test -s conftest.err],
-                [AS_VAR_SET([ac_var], [no])],
-                [AS_VAR_SET([ac_var], [yes])])],
-            [AS_VAR_SET([ac_var], [no])])
-    ])
-
-    AS_IF([test yes = AS_VAR_GET([ac_var])],
-        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_VAR_ATTRIBUTE_$1), 1,
-            [Define to 1 if the system has the `$1' variable attribute])], [])
-
-    AS_VAR_POPDEF([ac_var])
-])
diff --git a/src/conf_post.h b/src/conf_post.h
index 98ff12e..5c332a0 100644
--- a/src/conf_post.h
+++ b/src/conf_post.h
@@ -51,10 +51,21 @@ typedef bool bool_bf;
 #endif
 #endif
 
-/* When not using Clang, assume its attributes and features are absent.  */
+/* Simulate __has_attribute on compilers that lack it.  It is used only
+   on arguments like alloc_size that are handled in this simulation.  */
 #ifndef __has_attribute
-# define __has_attribute(a) false
-#endif
+# define __has_attribute(a) __has_attribute_##a
+# define __has_attribute_alloc_size (4 < __GNUC__ + (3 <= __GNUC_MINOR__))
+# define __has_attribute_cleanup (3 < __GNUC__ + (4 <= __GNUC_MINOR__))
+# define __has_attribute_externally_visible \
+    (4 < __GNUC__ + (1 <= __GNUC_MINOR__))
+# define __has_attribute_no_address_safety_analysis false
+# define __has_attribute_no_sanitize_address \
+    (4 < __GNUC__ + (8 <= __GNUC_MINOR__))
+#endif
+
+/* Simulate __has_feature on compilers that lack it.  It is used only
+   to define ADDRESS_SANITIZER below.  */
 #ifndef __has_feature
 # define __has_feature(a) false
 #endif
@@ -222,9 +233,7 @@ extern int emacs_setenv_TZ (char const *);
 #define NO_INLINE
 #endif
 
-#if (__clang__								\
-     ? __has_attribute (externally_visible)				\
-     : (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1)))
+#if __has_attribute (externally_visible)
 #define EXTERNALLY_VISIBLE __attribute__((externally_visible))
 #else
 #define EXTERNALLY_VISIBLE
@@ -253,9 +262,7 @@ extern int emacs_setenv_TZ (char const *);
 # define ATTRIBUTE_MALLOC
 #endif
 
-#if (__clang__					\
-     ? __has_attribute (alloc_size)		\
-     : 4 < __GNUC__ + (3 <= __GNUC_MINOR__))
+#if __has_attribute (alloc_size)
 # define ATTRIBUTE_ALLOC_SIZE(args) __attribute__ ((__alloc_size__ args))
 #else
 # define ATTRIBUTE_ALLOC_SIZE(args)
@@ -278,8 +285,7 @@ extern int emacs_setenv_TZ (char const *);
 /* Attribute of functions whose code should not have addresses
    sanitized.  */
 
-#if (__has_attribute (no_sanitize_address) \
-     || 4 < __GNUC__ + (8 <= __GNUC_MINOR__))
+#if __has_attribute (no_sanitize_address)
 # define ATTRIBUTE_NO_SANITIZE_ADDRESS \
     __attribute__ ((no_sanitize_address)) ADDRESS_SANITIZER_WORKAROUND
 #elif __has_attribute (no_address_safety_analysis)
diff --git a/src/emacs-module.c b/src/emacs-module.c
index b5e044e..79a077b 100644
--- a/src/emacs-module.c
+++ b/src/emacs-module.c
@@ -35,8 +35,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* Feature tests.  */
 
-/* True if __attribute__ ((cleanup (...))) works, false otherwise.  */
-#ifdef HAVE_VAR_ATTRIBUTE_CLEANUP
+#if __has_attribute (cleanup)
 enum { module_has_cleanup = true };
 #else
 enum { module_has_cleanup = false };
-- 
2.5.0