Bug 60670 - omp.h may differ between multilibs
Summary: omp.h may differ between multilibs
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: libgomp (show other bugs)
Version: 4.9.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: patch, rejects-valid
Depends on:
Blocks:
 
Reported: 2014-03-26 14:23 UTC by Rainer Orth
Modified: 2017-11-15 00:49 UTC (History)
7 users (show)

See Also:
Host: *-*-solaris2.*, x86_64-unknown-linux-gnu
Target: *-*-solaris2.*, x86_64-unknown-linux-gnu
Build: *-*-solaris2.*, x86_64-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2015-02-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Orth 2014-03-26 14:23:01 UTC
It has been noticed that the libgomp omp.h header that is generated at build time
may differ between different multilibs.  E.g. 

* i386-pc-solaris2.11:

--- omp.h       2014-03-21 14:27:34.522529041 +0100
+++ ../amd64/libgomp/omp.h      2014-03-21 14:27:56.903263206 +0100
@@ -34,13 +34,13 @@
 typedef struct
 {
   unsigned char _x[48] 
-    __attribute__((__aligned__(4)));
+    __attribute__((__aligned__(8)));
 } omp_lock_t;
 
 typedef struct
 {
-  unsigned char _x[56] 
-    __attribute__((__aligned__(4)));
+  unsigned char _x[64] 
+    __attribute__((__aligned__(8)));
 } omp_nest_lock_t;
 #endif
 
* sparc-sun-solaris2.11:

--- omp.h       2014-03-21 15:26:13.738263000 +0100
+++ ../sparcv9/libgomp/omp.h    2014-03-21 15:26:49.743545700 +0100
@@ -39,7 +39,7 @@
 
 typedef struct
 {
-  unsigned char _x[56] 
+  unsigned char _x[64] 
     __attribute__((__aligned__(8)));
 } omp_nest_lock_t;
 #endif

* x86_64-unknown-linux-gnu:

--- omp.h       2014-03-21 19:02:19.351059178 +0100
+++ ../32/libgomp/omp.h 2014-03-21 19:02:30.432588699 +0100
@@ -39,8 +39,8 @@
 
 typedef struct
 {
-  unsigned char _x[16] 
-    __attribute__((__aligned__(8)));
+  unsigned char _x[12] 
+    __attribute__((__aligned__(4)));
 } omp_nest_lock_t;
 #endif
 
Unfortunately, only the version for the default multilib is installed.  Given
that variables of those types can be allocated by user code and size and/or
alignment can differ between different multilibs, this can lead to all sorts
of errors when the allocations performed are smaller than what libgomp assumes.

It seems like different per-multilib headers need to be installed into separate
hierarchies to handle this.

  Rainer
Comment 1 Richard Biener 2014-03-28 09:23:57 UTC
Or the header needs to include all variants with proper #ifdef-ery
Comment 2 ro@CeBiTec.Uni-Bielefeld.DE 2014-03-28 09:27:53 UTC
> --- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
> Or the header needs to include all variants with proper #ifdef-ery

This is difficult for a header generated per multilib at build time.

	Rainer
Comment 3 Tobias Burnus 2014-04-04 15:15:55 UTC
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #2)
> > --- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
> > Or the header needs to include all variants with proper #ifdef-ery
> 
> This is difficult for a header generated per multilib at build time.

A similar issue potentially arises for OpenMP's Fortran libraries under finclude/, i.e. omp_lib.h, omp_lib.f90, omp_lib.mod, omp_lib_kinds.mod.

Recall that the module files (.mod) are not preprocessed - and the .h files are not if one uses "include 'omp_lib'" (instead of "#include")

However, I think for currently used type there should be no problem (with all/most targets).
Comment 4 ro@CeBiTec.Uni-Bielefeld.DE 2014-04-04 15:20:24 UTC
> --- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
> (In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #2)
>> > --- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
>> > Or the header needs to include all variants with proper #ifdef-ery
>> 
>> This is difficult for a header generated per multilib at build time.
>
> A similar issue potentially arises for OpenMP's Fortran libraries under
> finclude/, i.e. omp_lib.h, omp_lib.f90, omp_lib.mod, omp_lib_kinds.mod.
>
> Recall that the module files (.mod) are not preprocessed - and the .h files are
> not if one uses "include 'omp_lib'" (instead of "#include")
>
> However, I think for currently used type there should be no problem (with
> all/most targets).

Right, at least on Solaris/SPARC and x86 the 32-bit and 64-bit
omp_lib.{h,mod} are identical.

	Rainer
Comment 5 Jakub Jelinek 2014-04-22 11:36:36 UTC
GCC 4.9.0 has been released
Comment 6 jacknagel 2014-06-12 19:12:55 UTC
This also happens on Darwin (i386/x86_64 multilib).
Comment 7 Jakub Jelinek 2014-07-16 13:29:28 UTC
GCC 4.9.1 has been released.
Comment 8 Jakub Jelinek 2014-10-30 10:39:38 UTC
GCC 4.9.2 has been released.
Comment 9 Eric Gallager 2015-01-09 23:22:02 UTC
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #2)
> > --- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
> > Or the header needs to include all variants with proper #ifdef-ery
> 
> This is difficult for a header generated per multilib at build time.
>

GNU diff can can merge two C/C++ header files with the proper ifdef-ery: see:
http://www.gnu.org/software/diffutils/manual/html_mono/diff.html#If-then-else
and also:
http://www.gnu.org/software/diffutils/manual/html_mono/diff.html#Detailed%20If-then-else

This could be done as part of the generation of the header at build time.
Comment 10 Andrew Pinski 2015-02-10 19:10:38 UTC
Or better yet change @OMP_LOCK_SIZE@ and @OMP_LOCK_ALIGN@ from being a configured generated to one which is done in a header file.  See how libffi handles this.

Confirmed.
Comment 11 Jakub Jelinek 2015-06-26 19:54:55 UTC
GCC 4.9.3 has been released.
Comment 12 Francois-Xavier Coudert 2016-05-03 16:20:50 UTC
The Fortran module files are not installed in the right place, and that leads to error upon compilation of valid user code (not finding the module files).

$ ls /usr/local/gfortran/lib/**/libgomp.dylib
/usr/local/gfortran/lib/i386/libgomp.dylib
/usr/local/gfortran/lib/libgomp.dylib

$ ls /usr/local/gfortran/lib/**/omp_lib.mod  
/usr/local/gfortran/lib/gcc/x86_64-apple-darwin15/6.1.0/finclude/omp_lib.mod


It’s because the Makefile machinery is wrong. libgomp/Makefile.am has:

    fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/finclude

while the corresponding libgfortran has:

fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/finclude

notice the extra $(MULTISUBDIR).
Comment 13 Francois-Xavier Coudert 2016-05-03 20:48:19 UTC
Showing how the module files are not found when a non default multilib is chosen:

$ gfortran a.f90 -fopenmp     
$ gfortran a.f90 -fopenmp -m32
a.f90:1:6:

   use omp_lib
      1
Fatal Error: Can't open module file ‘omp_lib.mod’ for reading at (1): No such file or directory
compilation terminated.
$ cat a.f90 
  use omp_lib
  end
Comment 14 Francois-Xavier Coudert 2016-05-03 20:53:50 UTC
Based on libgfortran build machinery, the following patch should be applied to libgomp:

Index: libgomp/Makefile.am
===================================================================
--- libgomp/Makefile.am	(revision 235843)
+++ libgomp/Makefile.am	(working copy)
@@ -10,7 +10,7 @@ config_path = @config_path@
 search_path = $(addprefix $(top_srcdir)/config/, $(config_path)) $(top_srcdir) \
 	      $(top_srcdir)/../include
 
-fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/finclude
+fincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)/finclude
 libsubincludedir = $(libdir)/gcc/$(target_alias)/$(gcc_version)/include
 
 vpath % $(strip $(search_path))
Comment 15 Francois-Xavier Coudert 2017-01-09 20:29:38 UTC
Author: fxcoudert
Date: Mon Jan  9 20:29:06 2017
New Revision: 244239

URL: https://gcc.gnu.org/viewcvs?rev=244239&root=gcc&view=rev
Log:
	PR libgomp/60670
	* Makefile.am: Make fincludedir multilib-aware.
	* Makefile.in: Regenerate.

Modified:
    trunk/libgomp/ChangeLog
    trunk/libgomp/Makefile.am
    trunk/libgomp/Makefile.in
Comment 16 Francois-Xavier Coudert 2017-01-09 20:34:56 UTC
The Fortran part is now fixed.
Comment 17 Radford Neal 2017-11-15 00:49:54 UTC
I'd like to add some urgency to getting this fixed.  

The problem with omp.h defining an incorrect omp_lock_t type shows up when gcc is installed with Homebrew (https://brew.sh) on macOS, for any modern 64-bit system, since the omp_lock_t type is set up for 32-bit builds.  It also shows up with the Rtools package for installing R on Windows.  It probably shows up in various other contexts too.  The consequence is that OpenMP doesn't work correctly, in ways that may well be non-obvious, and very hard to diagnose for anyone who doesn't realize what is going on.

One could say that these package providers ought to provide separate 32-bit and 64-bit versions of omp.h, but the fact is that they don't.  And it's not really unreasonable for them to think that omp.h will correctly define the types for both 32-bit and 64-bit builds - that's the way just about every other package works.  What you're doing with keeping omg_log_t "private" by defining it as a byte array with length filled in during the build is decidedly not a standard approach, and it's unsurprising that it ends up causing problems.  You ought to change to a different approach.

If that's not possible immediately, however, you should implement a kludge - just set @OMP_LOCK_SIZE@ to the maximum that it might be for any platform.