diff mbox

Resolve idempotency issue with libgomp's config.h/libgomp.h

Message ID 87a8ko3ea0.fsf@hertz.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge April 20, 2016, 7:55 a.m. UTC 
Hi!

Some time ago, by chance, I had found that libgomp's config.h/libgomp.h
produce different results depending in which combination/order they're
being included.  As I remember, this caused different results in/with the
following libgomp.h section:

    #if !defined (HAVE_ATTRIBUTE_VISIBILITY) \
        || !defined (HAVE_ATTRIBUTE_ALIAS) \
        || !defined (HAVE_AS_SYMVER_DIRECTIVE) \
        || !defined (PIC) \
        || !defined (HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT)
    # undef LIBGOMP_GNU_SYMBOL_VERSIONING
    #endif

..., and then caused different results for the following users of
LIBGOMP_GNU_SYMBOL_VERSIONING.  Maybe this should be fixed differently
(to make inclusion order of these files idempotent), but will the
following be OK for now (for gcc-6-branch and/or trunk?) to fix the
status quo?

commit 075063cb8611bb479289b6e0c80a54d94fc3a14e
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue Nov 10 16:56:01 2015 +0100

    Resolve idempotency issue with libgomp's config.h/libgomp.h
    
    	libgomp/
    	* configure.ac: Instantiate AH_TOP to add an #include guard.
    	* config.h.in: Regenerate.
    	* configure: Regenerate.
    	* libgomp.h: Include "config.h" early.
    	* oacc-cuda.c: Include "libgomp.h" early, don't include
    	"config.h".
    	* oacc-int.h: Likewise.
    	* oacc-mem.c: Likewise.
    	* plugin/plugin-nvptx.c: Likewise.
    	* target.c: Likewise.
---
 libgomp/config.h.in           | 6 ++++++
 libgomp/configure             | 5 +++--
 libgomp/configure.ac          | 5 +++++
 libgomp/libgomp.h             | 8 ++++++--
 libgomp/oacc-cuda.c           | 3 +--
 libgomp/oacc-int.h            | 2 +-
 libgomp/oacc-mem.c            | 3 +--
 libgomp/plugin/plugin-nvptx.c | 2 +-
 libgomp/target.c              | 1 -
 9 files changed, 24 insertions(+), 11 deletions(-)



Grüße
 Thomas

Comments

Jakub Jelinek April 20, 2016, 10:25 a.m. UTC | #1 
On Wed, Apr 20, 2016 at 09:55:35AM +0200, Thomas Schwinge wrote:
> diff --git libgomp/config.h.in libgomp/config.h.in
> index 226ac53..1ef51ca 100644
> --- libgomp/config.h.in
> +++ libgomp/config.h.in
> @@ -1,5 +1,11 @@
>  /* config.h.in.  Generated from configure.ac by autoheader.  */
>  
> +
> +  #ifdef LIBGOMP_H
> +  # error Must not #include "config.h" after #include "libgomp.h".
> +  #endif
> +
> +
>  /* Define to 1 if the target assembler supports .symver directive. */
>  #undef HAVE_AS_SYMVER_DIRECTIVE

> --- libgomp/libgomp.h
> +++ libgomp/libgomp.h
> @@ -33,7 +33,12 @@
>     that are part of the external ABI, and the lower case prefix "gomp"
>     is used group items that are completely private to the library.  */
>  
> -#ifndef LIBGOMP_H 
> +#ifndef LIBGOMP_H
> +/* We #include "config.h" early, before we #define LIBGOMP_H, so that we can
> +   use the latter to check in "config.h" that it's not being included again,
> +   which might conflict with configuration changes done further down in
> +   libgomp.h.  */
> +#include "config.h"
>  #define LIBGOMP_H 1

The above breaks the multiple inclusion guards of libgomp.h, the
preprocessor will need to treat them as normal macros.
So IMNSHO it would be better to just use a different macro for this, keep
config.h included where it is now in libgomp.h and just make sure the macro
is defined after it.
Either use one of the many preexisting macros, like gomp_alloca, ...,
REFCOUNT_INFINITY, ... _LIBGOMP_OMP_LOCK_DEFINED, attribute_hidden, ...,
ialias, ..., or add one specially for this purpose.

Otherwise it is reasonable, but only for trunk and 6.2.

	Jakub
diff mbox

Patch

diff --git libgomp/config.h.in libgomp/config.h.in
index 226ac53..1ef51ca 100644
--- libgomp/config.h.in
+++ libgomp/config.h.in
@@ -1,5 +1,11 @@ 
 /* config.h.in.  Generated from configure.ac by autoheader.  */
 
+
+  #ifdef LIBGOMP_H
+  # error Must not #include "config.h" after #include "libgomp.h".
+  #endif
+
+
 /* Define to 1 if the target assembler supports .symver directive. */
 #undef HAVE_AS_SYMVER_DIRECTIVE
 
diff --git libgomp/configure libgomp/configure
index 8d03eb6..327bd1b 100755
--- libgomp/configure
+++ libgomp/configure
@@ -2592,6 +2592,7 @@  ac_compiler_gnu=$ac_cv_c_compiler_gnu
 ac_config_headers="$ac_config_headers config.h"
 
 
+
 # -------
 # Options
 # -------
@@ -11145,7 +11146,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11148 "configure"
+#line 11149 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11251,7 +11252,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11254 "configure"
+#line 11255 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git libgomp/configure.ac libgomp/configure.ac
index 2e41ca8..b9a665f 100644
--- libgomp/configure.ac
+++ libgomp/configure.ac
@@ -4,6 +4,11 @@ 
 AC_PREREQ(2.64)
 AC_INIT([GNU Offloading and Multi Processing Runtime Library], 1.0,,[libgomp])
 AC_CONFIG_HEADER(config.h)
+AH_TOP([
+  #ifdef LIBGOMP_H
+  # error Must not #include "config.h" after #include "libgomp.h".
+  #endif
+])
 
 # -------
 # Options
diff --git libgomp/libgomp.h libgomp/libgomp.h
index 664e76b..6a05bbc 100644
--- libgomp/libgomp.h
+++ libgomp/libgomp.h
@@ -33,7 +33,12 @@ 
    that are part of the external ABI, and the lower case prefix "gomp"
    is used group items that are completely private to the library.  */
 
-#ifndef LIBGOMP_H 
+#ifndef LIBGOMP_H
+/* We #include "config.h" early, before we #define LIBGOMP_H, so that we can
+   use the latter to check in "config.h" that it's not being included again,
+   which might conflict with configuration changes done further down in
+   libgomp.h.  */
+#include "config.h"
 #define LIBGOMP_H 1
 
 #ifndef _LIBGOMP_CHECKING_
@@ -41,7 +46,6 @@ 
 #define _LIBGOMP_CHECKING_ 0
 #endif
 
-#include "config.h"
 #include "gstdint.h"
 #include "libgomp-plugin.h"
 
diff --git libgomp/oacc-cuda.c libgomp/oacc-cuda.c
index 86a2a77..115697d 100644
--- libgomp/oacc-cuda.c
+++ libgomp/oacc-cuda.c
@@ -26,9 +26,8 @@ 
    see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include "openacc.h"
-#include "config.h"
 #include "libgomp.h"
+#include "openacc.h"
 #include "oacc-int.h"
 
 void *
diff --git libgomp/oacc-int.h libgomp/oacc-int.h
index db0a937..7fd9e95 100644
--- libgomp/oacc-int.h
+++ libgomp/oacc-int.h
@@ -38,8 +38,8 @@ 
 #ifndef OACC_INT_H
 #define OACC_INT_H 1
 
+#include "libgomp.h"
 #include "openacc.h"
-#include "config.h"
 #include <stddef.h>
 #include <stdbool.h>
 #include <stdarg.h>
diff --git libgomp/oacc-mem.c libgomp/oacc-mem.c
index ce1905c..0d98a7f 100644
--- libgomp/oacc-mem.c
+++ libgomp/oacc-mem.c
@@ -26,9 +26,8 @@ 
    see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include "openacc.h"
-#include "config.h"
 #include "libgomp.h"
+#include "openacc.h"
 #include "gomp-constants.h"
 #include "oacc-int.h"
 #include <stdint.h>
diff --git libgomp/plugin/plugin-nvptx.c libgomp/plugin/plugin-nvptx.c
index 4b57833..fc5f298 100644
--- libgomp/plugin/plugin-nvptx.c
+++ libgomp/plugin/plugin-nvptx.c
@@ -31,8 +31,8 @@ 
    is not clear as to what that state might be.  Or how one might
    propagate it from one thread to another.  */
 
+#include "libgomp.h"
 #include "openacc.h"
-#include "config.h"
 #include "libgomp-plugin.h"
 #include "oacc-plugin.h"
 #include "gomp-constants.h"
diff --git libgomp/target.c libgomp/target.c
index e2dd0e0..dd6f74d 100644
--- libgomp/target.c
+++ libgomp/target.c
@@ -25,7 +25,6 @@ 
 
 /* This file contains the support of offloading.  */
 
-#include "config.h"
 #include "libgomp.h"
 #include "oacc-plugin.h"
 #include "oacc-int.h"