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
