[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#888566: vim FTBFS on alpha: vim segfaults in the build



On Sat, Feb 24, 2018 at 07:13:02PM +0100, Aurelien Jarno wrote:
> On 2018-01-28 21:05, James McCoy wrote:
> > Control: reassign -1 src:glibc 2.24-2
> > Control: affects -1 src:vim
> > 
> > On Sat, Jan 27, 2018 at 02:37:21PM -0500, James McCoy wrote:
> > > On Sat, Jan 27, 2018 at 05:50:16PM +1300, Michael Cree wrote:
> > > > [snip useful details]
> > > > 
> > > > Four of those bytes difference are due to the field b_ino in the
> > > > struct (of type ino_t).  And indeed examining the build log [1]
> > > > one sees option.c is compiled with -DFILE_OFFSET_BITS=64 but
> > > > misc1.c is NOT compiled with that option, hence the difference
> > > > in offsets to fields in the buf_T struct.
> > > 
> > > It looks like you came to the same conclusion as we did in #827319.  Vim
> > > uses autoconf's AC_SYS_LARGEFILE to determine whether _FILE_OFFSET_BITS
> > > needs to be set (and decides it doesn't), however the Perl bindings
> > > always force _FILE_OFFSET_BITS=64[0].
> > 
> > Note that Vim handles this via config.h, so it's not actually evident
> > from the compile commands whether an individual module is being built
> > with -D_FILE_OFFSET_BITS=64.  Having looked through Vim's source, I can
> > confirm that Vim does properly ensure config.h is loaded before any
> > system headers, though.
> > 
> > After some discussion on IRC, bunk pointed out this is an issue with
> > glibc on alpha and kfreebsd-*.
> > 
> > Alpha
> > -----
> > 
> > AC_SYS_LARGEFILE determines whether -D_FILE_OFFSET_BITS=64 is needed by
> > checking whether off_t is 64-bit on its own.  For alpha, this is true
> > because
> > 
> > usr/include/alpha-linux-gnu/bits/typesizes.h
> > 36:#define __OFF_T_TYPE         __SLONGWORD_TYPE
> > 67:#define __OFF_T_MATCHES_OFF64_T      1
> >
> > On the other hand, ino_t is always defined to be 32-bits
> > 
> > usr/include/alpha-linux-gnu/bits/typesizes.h
> > 32:#define __INO_T_TYPE         __U32_TYPE
> > 
> > and struct stat's st_ino varies type based on whether
> > __USE_FILE_OFFSET64 is defined
> > 
> > usr/include/alpha-linux-gnu/bits/stat.h
> > 69:struct stat
> > 70-  {
> > 71-    __dev_t st_dev;          /* Device.  */
> > 72-#ifdef __USE_FILE_OFFSET64
> > 73-    __ino64_t st_ino;                /* File serial number.  */
> > 74-#else
> > 75-    __ino_t st_ino;          /* File serial number.  */
> > 76-    int __pad0;                      /* 64-bit st_ino.  */
> > 
> > So, st_ino is only 64-bit when __USE_FILE_OFFSET64 is set, but that won't
> > typically be the case because off_t is _always_ 64-bit.
> 
> -D_FILE_OFFSET_BITS=64 changes a lot of types, and not only struct stat.
> It also causes for example struct statfs or struct dirent to change. It
> looks to me that the issue is to decide to pass -D_FILE_OFFSET_BITS=64
> depending on the size of off_t,

Which has been the mechanism behind autoconf's AC_SYS_LARGEFILE for
quite a while.

> while many more things are affected. It
> should not hurt to always pass -D_FILE_OFFSET_BITS=64.

Many projects rely on AC_SYS_LARGEFILE to determine whether and which
flags need to be set.  If AC_SYS_LARGEFILE determines the flag isn't
necessary, but some dependency advertises it, then it's easy to have
modules compiled with different settings, causing subtle breakage.

> In any case it's an existing interface, that probably wasn't done
> the best way on alpha as it's one of the first 64-bit platforms in
> glibc. The structures can't be easily changed without breaking the
> world. It should however be possible to use symbol versioning to
> provide a new version of all the functions affected by this change.
> If some alpha porters have the motivation to do so, it should be
> coordinated upstream.

Hopefully alpha (and kfreebsd-*) are the only platforms affected.

> Otherwise vim should have to be fixed.

Vim now includes -D_FILE_OFFSET_BITS=64 if AC_SYS_LARGEFILE determines
it is needed or any libraries it links against include that in their
exported CFLAGS, so it handles this.  I was just hoping there was
something broader that could be done, since it was troublesome to track
down.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB


Reply to: