diff mbox

Latest GCC fails to build glibc

Message ID 5559D484.8090002@redhat.com
State Committed
Headers

Commit Message

Florian Weimer May 18, 2015, 12:01 p.m. UTC 
  On 05/14/2015 10:59 PM, Steve Ellcey  wrote:
> A recent change to GCC (Fix to PR middle-end/66110) has caused glibc to
> fail to build.  When compiling the latest glibc with the latest GCC I
> get the error below.  I don't know if this is a GCC problem or a glibc
> problem.  I notice that when Richard checked this in he also added the
> use of -Wno-strict-aliasing to the compilation of one of the GCC files.
> Maybe we need the same thing when compiling memstream.c?
> 
> Steve Ellcey
> sellcey@imgtec.com
> 
> 
> In file included from memstream.c:18:0:
> memstream.c: In function 'open_memstream':
> memstream.c:92:22: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
>    _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf) = &_IO_mem_jumps;

Steve, could you try the attached patch, please?

An alternative fix for some of these issues would be to use the
cast-free member access where the type of the outer object is known from
context, getting rid of _IO_JUMPS.

Comments

Steve Ellcey May 18, 2015, 7:56 p.m. UTC | #1 
On Mon, 2015-05-18 at 14:01 +0200, Florian Weimer wrote:

> Steve, could you try the attached patch, please?
> 
> An alternative fix for some of these issues would be to use the
> cast-free member access where the type of the outer object is known from
> context, getting rid of _IO_JUMPS.

That patch fixed the libio strict aliasing problems for me.  There are
still problems with the resolv/res_hconf.c and inet/rcmd.c files, but
this patch took care of all of the libio build problems.

Steve Ellcey
Steve Ellcey May 20, 2015, 3:31 p.m. UTC | #2 
On Mon, 2015-05-18 at 14:01 +0200, Florian Weimer wrote:

> > 
> > In file included from memstream.c:18:0:
> > memstream.c: In function 'open_memstream':
> > memstream.c:92:22: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
> >    _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf) = &_IO_mem_jumps;
> 
> Steve, could you try the attached patch, please?
> 
> An alternative fix for some of these issues would be to use the
> cast-free member access where the type of the outer object is known from
> context, getting rid of _IO_JUMPS.

Does anyone else have any comments on Florian's patch to get rid of the
strict-warning errors from the libio directory?  I compiled it and
verified that it worked for me but I don't really feel comfortable
approving the patch as I am not very familiar with the code in this
directory and am not sure how others feel about this particular approach
to getting rid of the errors.

Steve Ellcey
sellcey@imgtec.com
Carlos O'Donell May 20, 2015, 4:03 p.m. UTC | #3 
On 05/20/2015 11:31 AM, Steve Ellcey wrote:
> On Mon, 2015-05-18 at 14:01 +0200, Florian Weimer wrote:
> 
>>>
>>> In file included from memstream.c:18:0:
>>> memstream.c: In function 'open_memstream':
>>> memstream.c:92:22: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
>>>    _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf) = &_IO_mem_jumps;
>>
>> Steve, could you try the attached patch, please?
>>
>> An alternative fix for some of these issues would be to use the
>> cast-free member access where the type of the outer object is known from
>> context, getting rid of _IO_JUMPS.
> 
> Does anyone else have any comments on Florian's patch to get rid of the
> strict-warning errors from the libio directory?  I compiled it and
> verified that it worked for me but I don't really feel comfortable
> approving the patch as I am not very familiar with the code in this
> directory and am not sure how others feel about this particular approach
> to getting rid of the errors.

Florian's patch looks good to me.

The offsetof trick is one way around the strict aliasing violation.

In this case it works well.

Cheers,
Carlos.
Andreas Schwab May 20, 2015, 4:17 p.m. UTC | #4 
Florian Weimer <fweimer@redhat.com> writes:

> +/* Essentially ((TYPE *) THIS)->MEMBER, but avoiding the aliasing
> +   violation in case THIS has a different pointer type.  */
> +#define _IO_CAST_FIELD_ACCESS(THIS, TYPE, MEMBER) \
> +  (*(_IO_MEMBER_TYPE (TYPE, MEMBER) *)(((char *) (THIS)) \
> +  + offsetof(TYPE, MEMBER)))

Please fix the indentation to match nesting.

Andreas.
Florian Weimer May 22, 2015, 10:05 a.m. UTC | #5 
On 05/20/2015 06:03 PM, Carlos O'Donell wrote:
> On 05/20/2015 11:31 AM, Steve Ellcey wrote:
>> On Mon, 2015-05-18 at 14:01 +0200, Florian Weimer wrote:
>>
>>>>
>>>> In file included from memstream.c:18:0:
>>>> memstream.c: In function 'open_memstream':
>>>> memstream.c:92:22: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
>>>>    _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf) = &_IO_mem_jumps;
>>>
>>> Steve, could you try the attached patch, please?
>>>
>>> An alternative fix for some of these issues would be to use the
>>> cast-free member access where the type of the outer object is known from
>>> context, getting rid of _IO_JUMPS.
>>
>> Does anyone else have any comments on Florian's patch to get rid of the
>> strict-warning errors from the libio directory?  I compiled it and
>> verified that it worked for me but I don't really feel comfortable
>> approving the patch as I am not very familiar with the code in this
>> directory and am not sure how others feel about this particular approach
>> to getting rid of the errors.
> 
> Florian's patch looks good to me.
> 
> The offsetof trick is one way around the strict aliasing violation.
> 
> In this case it works well.

Thanks, committed.
diff mbox

Patch

From aa8c921e167e7dddaf935cac7970a6d6eb972017 Mon Sep 17 00:00:00 2001
Message-Id: <aa8c921e167e7dddaf935cac7970a6d6eb972017.1431950456.git.fweimer@redhat.com>
From: Florian Weimer <fweimer@redhat.com>
Date: Mon, 18 May 2015 13:58:54 +0200
Subject: [PATCH] Avoid some aliasing violations in libio
To: libc-alpha@sourceware.org

---
 ChangeLog           | 18 ++++++++++++++++++
 libio/fileops.c     | 18 +++++++++---------
 libio/freopen.c     |  4 ++--
 libio/freopen64.c   |  2 +-
 libio/iofopen.c     |  4 ++--
 libio/iofopncook.c  |  2 +-
 libio/iofwide.c     |  2 +-
 libio/libioP.h      | 23 +++++++++++++++++++----
 libio/memstream.c   |  2 +-
 libio/oldiofdopen.c |  2 +-
 libio/oldiofopen.c  |  2 +-
 libio/oldiopopen.c  |  2 +-
 12 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4de8a25..e5d1878 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@ 
+2015-05-18  Florian Weimer  <fweimer@redhat.com>
+
+	* libio/libioP.h (_IO_MEMBER_TYPE, _IO_CAST_FIELD_ACCESS)
+	(_IO_JUMPS_FILE_plus): New.
+	(_IO_WIDE_JUMPS, _IO_CHECK_WIDE, _IO_JUMPS_FUNC): Use
+	_IO_CAST_FIELD_ACCESS.
+	* libio/fileops.c (libc_hidden_def, _IO_file_setbuf_mmap,
+	mmap_remap_check, decide_maybe_mmap): Use _IO_JUMPS_FILE_plus.
+	* libio/freopen.c (freopen): Likewise.
+	* libio/freopen64.c (freopen64): Likewise.
+	* libio/iofopen.c (__fopen_maybe_mmap): Likewise.
+	* libio/iofopncook.c (_IO_old_fopencookie): Likewise.
+	* libio/iofwide.c (_IO_fwide): Likewise.
+	* libio/memstream.c (open_memstream): Likewise.
+	* libio/oldiofdopen.c (_IO_old_fdopen): Likewise.
+	* libio/oldiofopen.c (_IO_old_fopen): Likewise.
+	* libio/oldiopopen.c (_IO_old_popen): Likewise.
+
 2015-05-18  Arjun Shankar  <arjun.is@lostca.se>
 
 	* include/stdio.h: Define __need_wint_t.
diff --git a/libio/fileops.c b/libio/fileops.c
index 2427320..9668024 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -414,7 +414,7 @@  _IO_new_file_fopen (_IO_FILE *fp, const char *filename, const char *mode,
 	    &result->_wide_data->_IO_state;
 
 	  /* From now on use the wide character callback functions.  */
-	  ((struct _IO_FILE_plus *) fp)->vtable = fp->_wide_data->_wide_vtable;
+	  _IO_JUMPS_FILE_plus (fp) = fp->_wide_data->_wide_vtable;
 
 	  /* Set the mode now.  */
 	  result->_mode = 1;
@@ -466,7 +466,7 @@  _IO_file_setbuf_mmap (_IO_FILE *fp, char *p, _IO_ssize_t len)
   _IO_FILE *result;
 
   /* Change the function table.  */
-  _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps;
+  _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
   fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
 
   /* And perform the normal operation.  */
@@ -475,7 +475,7 @@  _IO_file_setbuf_mmap (_IO_FILE *fp, char *p, _IO_ssize_t len)
   /* If the call failed, restore to using mmap.  */
   if (result == NULL)
     {
-      _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps_mmap;
+      _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps_mmap;
       fp->_wide_data->_wide_vtable = &_IO_wfile_jumps_mmap;
     }
 
@@ -703,9 +703,9 @@  mmap_remap_check (_IO_FILE *fp)
       fp->_IO_buf_base = fp->_IO_buf_end = NULL;
       _IO_setg (fp, NULL, NULL, NULL);
       if (fp->_mode <= 0)
-	_IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps;
+	_IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
       else
-	_IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_wfile_jumps;
+	_IO_JUMPS_FILE_plus (fp) = &_IO_wfile_jumps;
       fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
 
       return 1;
@@ -773,9 +773,9 @@  decide_maybe_mmap (_IO_FILE *fp)
 	      fp->_offset = st.st_size;
 
 	      if (fp->_mode <= 0)
-		_IO_JUMPS ((struct _IO_FILE_plus *)fp) = &_IO_file_jumps_mmap;
+		_IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps_mmap;
 	      else
-		_IO_JUMPS ((struct _IO_FILE_plus *)fp) = &_IO_wfile_jumps_mmap;
+		_IO_JUMPS_FILE_plus (fp) = &_IO_wfile_jumps_mmap;
 	      fp->_wide_data->_wide_vtable = &_IO_wfile_jumps_mmap;
 
 	      return;
@@ -786,9 +786,9 @@  decide_maybe_mmap (_IO_FILE *fp)
   /* We couldn't use mmap, so revert to the vanilla file operations.  */
 
   if (fp->_mode <= 0)
-    _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps;
+    _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
   else
-    _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_wfile_jumps;
+    _IO_JUMPS_FILE_plus (fp) = &_IO_wfile_jumps;
   fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
 }
 
diff --git a/libio/freopen.c b/libio/freopen.c
index 91cde19..035fa1f 100644
--- a/libio/freopen.c
+++ b/libio/freopen.c
@@ -59,14 +59,14 @@  freopen (filename, mode, fp)
 	 to the old libio may be passed into shared C library and wind
 	 up here. */
       _IO_old_file_close_it (fp);
-      _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_old_file_jumps;
+      _IO_JUMPS_FILE_plus (fp) = &_IO_old_file_jumps;
       result = _IO_old_file_fopen (fp, gfilename, mode);
     }
   else
 #endif
     {
       _IO_file_close_it (fp);
-      _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps;
+      _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
       if (_IO_vtable_offset (fp) == 0 && fp->_wide_data != NULL)
 	fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
       result = _IO_file_fopen (fp, gfilename, mode, 1);
diff --git a/libio/freopen64.c b/libio/freopen64.c
index 77c83cf..fc6ccb1 100644
--- a/libio/freopen64.c
+++ b/libio/freopen64.c
@@ -50,7 +50,7 @@  freopen64 (filename, mode, fp)
 			   ? fd_to_filename (fd) : filename);
   fp->_flags2 |= _IO_FLAGS2_NOCLOSE;
   _IO_file_close_it (fp);
-  _IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps;
+  _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps;
   if (_IO_vtable_offset (fp) == 0 && fp->_wide_data != NULL)
     fp->_wide_data->_wide_vtable = &_IO_wfile_jumps;
   result = _IO_file_fopen (fp, gfilename, mode, 0);
diff --git a/libio/iofopen.c b/libio/iofopen.c
index c427ffb..be2bbb6 100644
--- a/libio/iofopen.c
+++ b/libio/iofopen.c
@@ -46,9 +46,9 @@  __fopen_maybe_mmap (_IO_FILE *fp)
 	 vanilla file operations and reset the jump table accordingly.  */
 
       if (fp->_mode <= 0)
-	_IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_file_jumps_maybe_mmap;
+	_IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps_maybe_mmap;
       else
-	_IO_JUMPS ((struct _IO_FILE_plus *) fp) = &_IO_wfile_jumps_maybe_mmap;
+	_IO_JUMPS_FILE_plus (fp) = &_IO_wfile_jumps_maybe_mmap;
       fp->_wide_data->_wide_vtable = &_IO_wfile_jumps_maybe_mmap;
     }
 #endif
diff --git a/libio/iofopncook.c b/libio/iofopncook.c
index 54ae413..b845d29 100644
--- a/libio/iofopncook.c
+++ b/libio/iofopncook.c
@@ -269,7 +269,7 @@  _IO_old_fopencookie (cookie, mode, io_functions)
 
   ret = _IO_fopencookie (cookie, mode, io_functions);
   if (ret != NULL)
-    _IO_JUMPS ((struct _IO_FILE_plus *) ret) = &_IO_old_cookie_jumps;
+    _IO_JUMPS_FILE_plus (ret) = &_IO_old_cookie_jumps;
 
   return ret;
 }
diff --git a/libio/iofwide.c b/libio/iofwide.c
index a8969e5..0c175d1 100644
--- a/libio/iofwide.c
+++ b/libio/iofwide.c
@@ -184,7 +184,7 @@  _IO_fwide (fp, mode)
 #endif
 
       /* From now on use the wide character callback functions.  */
-      ((struct _IO_FILE_plus *) fp)->vtable = fp->_wide_data->_wide_vtable;
+      _IO_JUMPS_FILE_plus (fp) = fp->_wide_data->_wide_vtable;
     }
 
   /* Set the mode now.  */
diff --git a/libio/libioP.h b/libio/libioP.h
index d8604ca..784a47b 100644
--- a/libio/libioP.h
+++ b/libio/libioP.h
@@ -32,6 +32,8 @@ 
 
    FIXME: All of the C++ cruft eventually needs to go away.  */
 
+#include <stddef.h>
+
 #include <errno.h>
 #ifndef __set_errno
 # define __set_errno(Val) errno = (Val)
@@ -104,17 +106,30 @@  extern "C" {
 # define _IO_JUMPS_OFFSET 0
 #endif
 
+/* Type of MEMBER in struct type TYPE.  */
+#define _IO_MEMBER_TYPE(TYPE, MEMBER) __typeof__ (((TYPE){}).MEMBER)
+
+/* Essentially ((TYPE *) THIS)->MEMBER, but avoiding the aliasing
+   violation in case THIS has a different pointer type.  */
+#define _IO_CAST_FIELD_ACCESS(THIS, TYPE, MEMBER) \
+  (*(_IO_MEMBER_TYPE (TYPE, MEMBER) *)(((char *) (THIS)) \
+  + offsetof(TYPE, MEMBER)))
+
 #define _IO_JUMPS(THIS) (THIS)->vtable
-#define _IO_WIDE_JUMPS(THIS) ((struct _IO_FILE *) (THIS))->_wide_data->_wide_vtable
-#define _IO_CHECK_WIDE(THIS) (((struct _IO_FILE *) (THIS))->_wide_data != NULL)
+#define _IO_JUMPS_FILE_plus(THIS) \
+  _IO_CAST_FIELD_ACCESS ((THIS), struct _IO_FILE_plus, vtable)
+#define _IO_WIDE_JUMPS(THIS) \
+  _IO_CAST_FIELD_ACCESS ((THIS), struct _IO_FILE, _wide_data)->_wide_vtable
+#define _IO_CHECK_WIDE(THIS) \
+  (_IO_CAST_FIELD_ACCESS ((THIS), struct _IO_FILE, _wide_data) != NULL)
 
 #if _IO_JUMPS_OFFSET
 # define _IO_JUMPS_FUNC(THIS) \
- (*(struct _IO_jump_t **) ((void *) &_IO_JUMPS ((struct _IO_FILE_plus *) (THIS)) \
+ (*(struct _IO_jump_t **) ((void *) &_IO_JUMPS_FILE_plus (THIS) \
 			   + (THIS)->_vtable_offset))
 # define _IO_vtable_offset(THIS) (THIS)->_vtable_offset
 #else
-# define _IO_JUMPS_FUNC(THIS) _IO_JUMPS ((struct _IO_FILE_plus *) (THIS))
+# define _IO_JUMPS_FUNC(THIS) _IO_JUMPS_FILE_plus (THIS)
 # define _IO_vtable_offset(THIS) 0
 #endif
 #define _IO_WIDE_JUMPS_FUNC(THIS) _IO_WIDE_JUMPS(THIS)
diff --git a/libio/memstream.c b/libio/memstream.c
index e1c5434..81bfe6d 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -89,7 +89,7 @@  open_memstream (bufloc, sizeloc)
       return NULL;
     }
   _IO_init (&new_f->fp._sf._sbf._f, 0);
-  _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp._sf._sbf) = &_IO_mem_jumps;
+  _IO_JUMPS_FILE_plus (&new_f->fp._sf._sbf) = &_IO_mem_jumps;
   _IO_str_init_static_internal (&new_f->fp._sf, buf, _IO_BUFSIZ, buf);
   new_f->fp._sf._sbf._f._flags &= ~_IO_USER_BUF;
   new_f->fp._sf._s._allocate_buffer = (_IO_alloc_type) malloc;
diff --git a/libio/oldiofdopen.c b/libio/oldiofdopen.c
index e0d5354..e068ec7 100644
--- a/libio/oldiofdopen.c
+++ b/libio/oldiofdopen.c
@@ -113,7 +113,7 @@  _IO_old_fdopen (fd, mode)
   new_f->fp.file._file._lock = &new_f->lock;
 #endif
   _IO_old_init (&new_f->fp.file._file, 0);
-  _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp) = &_IO_old_file_jumps;
+  _IO_JUMPS_FILE_plus (&new_f->fp) = &_IO_old_file_jumps;
   _IO_old_file_init ((struct _IO_FILE_plus *) &new_f->fp);
 #if  !_IO_UNIFIED_JUMPTABLES
   new_f->fp.vtable = NULL;
diff --git a/libio/oldiofopen.c b/libio/oldiofopen.c
index dac424a..a90a601 100644
--- a/libio/oldiofopen.c
+++ b/libio/oldiofopen.c
@@ -52,7 +52,7 @@  _IO_old_fopen (filename, mode)
   new_f->fp.file._file._lock = &new_f->lock;
 #endif
   _IO_old_init (&new_f->fp.file._file, 0);
-  _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fp) = &_IO_old_file_jumps;
+  _IO_JUMPS_FILE_plus (&new_f->fp) = &_IO_old_file_jumps;
   _IO_old_file_init ((struct _IO_FILE_plus *) &new_f->fp);
 #if  !_IO_UNIFIED_JUMPTABLES
   new_f->fp.vtable = NULL;
diff --git a/libio/oldiopopen.c b/libio/oldiopopen.c
index f6bcf5d..fb4c7b8 100644
--- a/libio/oldiopopen.c
+++ b/libio/oldiopopen.c
@@ -215,7 +215,7 @@  _IO_old_popen (command, mode)
 #endif
   fp = &new_f->fpx.file.file._file;
   _IO_old_init (fp, 0);
-  _IO_JUMPS ((struct _IO_FILE_plus *) &new_f->fpx.file) = &_IO_old_proc_jumps;
+  _IO_JUMPS_FILE_plus (&new_f->fpx.file) = &_IO_old_proc_jumps;
   _IO_old_file_init ((struct _IO_FILE_plus *) &new_f->fpx.file);
 #if  !_IO_UNIFIED_JUMPTABLES
   new_f->fpx.file.vtable = NULL;
-- 
2.1.0