Bug#776010: dpkg-cross: Can generate broken binaries due to off_t vs LFS disconnect
Hi!
On Thu, 2015-01-22 at 19:15:36 +0000, Wookey wrote:
> +++ Guillem Jover [2015-01-22 18:22 +0100] wrote:
> > Some of the config.site files define a value for ac_cv_sizeof_off_t,
> > but that value is wrong as it depends on whether the package is going
> > to be built with LFS enabled (through AC_SYS_LARGEFILE for example).
> > Unfortunately this is unfixable through a config.site deployment as the
> > LFS checks are performed way later than when the config cache files have
> > been loaded, so the value cannot be assigned depending on LFS.
> >
> > Those values would need to be removed from the config.site files, and
> > packages that lack the value during cross-compilation would need to be
> > updated to use a more recent autoconf which is able to compute such
> > sizeof values even when cross-compiling.
>
> OK. that's easy enough to do. It tends to occur twice. Once unconditionally, and once in:
> # parted
> if [ "$PACKAGE" = "parted" -o "$PACKAGE_NAME" = "GNU parted" ]; then
> ac_cv_sizeof_off_t=8
> fi
>
> Does parted know if it is being compiled with LFS or not? I presume
> not, as LFS is a kernel feature, right(?), which may or may not be enabled.
LFS is both a kernel feature and a libc feature. On most old 32-bit
arches the LFS enabled functions are an additional set of functions and
syscalls complementing the non-LFS ones with names usually ending with
64, like fstat64() for fstat(); or fseeko() for fseek().
These are usually not exposed if not requested, for backward
compatibility reasons. There's two ways to expose them, one is to
just make them visible, by defining _LARGEFILE_SOURCE and
_LARGEFILE64_SOURCE, but then you need to clutter your source with
conditional #ifdef to call fstat64() if available or fstat(), and this
is one of the reasons this is deprecated now. The other preferable
way is to define _FILE_OFFSET_BITS=64, which transparently redirects
the non-LFS function declarations and data types with the LFS enabled
ones, so that when you call open() you are in fact calling open64()
for example, or when you use off_t you are in fact using off64_t.
Of course if you are using an off_t of size 4 but calling a 64-bit
version of the functions then bad-things-will-happen. So having a
mismatched ac_cv_sizeof_off_t around is bad. (The same as not
including <config.h> as the first thing in every source file, as
then you have mismatching function calls in different objects.)
So, yes parted should know. It either is being passed an explicit
_FILE_OFFSET_BITS=64 or is using AC_SYS_LARGEFILE, which would allow
disabling LFS with «./configure --disable-largefile».
But even if we assume it is currently being built with LFS, hardcoding
the off_t size in the config.site is not wise.
> I was just wondering if there was any justification for leaving this
> in. I presume it's just as wrong in this per-package conditional, as
> it is 'bare'?
Yes.
> There are a lot of values in dpkg-cross site.config that _could_ be
> wrong. The setting are set are set for 'debian, with this glibc and
> standard kernel'. Is that in fact a fixed value for LFS? (which would
> be a possible excuse for leaving it in).
No, LFS does not have a fixed value, it's selectable at build time, as
mentioned above. The rest should be in principle constant per ABI,
except for when we change the ABI, remember the «long long double»
transition recently.
> But we shouldn't be keeping any values which can in fact be determined
> at build time, even when crossing, successfully. And it seem that
> AC_SYS_LARGEFILE suplies this now.
Yes, and with newer autotools the AC_CHECK_SIZEOF macros are
cross-build friendly so they do not need to be cached. But that
requires packages to be switched to use new autootols.
> And indeed current parted does use AC_SYS_LARGEFILE to set this so the
> above stanza is no longer needed.
>
> I've fixed all that. Every other variable in this package could do
> with similar review...
I skimmed over them before filing the bug, and seemed more or less
fine. Ideally we would not need any of the sizeof ones, but packages
still using old autotools might FTCBFS if removed.
Thanks,
Guillem
Reply to: