Monday 30 May 2022

Re: LFS_CFLAGS on 32bits architectures

Hi Andreas,

On Mon, May 30, 2022 at 02:16:59PM +0000, Andreas Hasenack wrote:
> on a pi3 (armhf):

> $ getconf LFS_CFLAGS
> -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64

> This only returns a value where it's needed, aka, 32bits (AFAIK).

> Shouldn't this be part of our default set of CFLAGS on 32bits architectures?

In principle, yes. The problem is, it is ABI-breaking to add this to the
default set of CFLAGS on an architecture after it has been bootstrapped.

There are libraries which expose either C/Linux types or their own types in
function prototypes, where those types are of different sizes depending on
the LFS defines in use. IIRC a classic example of this is zlib.

So because i386 and armhf (and likewise all other 32-bit Debian
architectures to date) have been bootstrapped without LFS_CFLAGS exported to
dpkg-buildflags by default, we can't add it now without introducing forced
ABI changes on an indeterminate number of libraries. Those libraries SHOULD
all be made to transition to LFS_CFLAGS, but we have to know which libraries
have their ABIs changing and plan transitions for them rather than just
flipping the switch.

If someone were able to do the necessary static analysis over the archive to
identify all of the affected libraries, then I think there's a strong case
to be made that we should do them all at once and then change the default
flags, because that would then fix the problem for any new libraries that
are introduced in the future.

FYI this issue is mentioned in the dpkg-buildflags manpage:

FEATURE AREAS
[...]
future
Several compile-time options (detailed below) can be used to enable
features that should be enabled by default, but cannot due to backwards
compatibility reasons.

lfs This setting (disabled by default) enables Large File Support on
32-bit architectures where their ABI does not include LFS by
default, by adding -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 to
CPPFLAGS.

> I experimented a few things, and this got it building again:
> --- a/debian/rules
> +++ b/debian/rules
> @@ -10,6 +10,10 @@ export DH_GOLANG_INSTALL_ALL := 1
> # Skip integration tests when building package: they need docker images.
> export ADSYS_SKIP_INTEGRATION_TESTS=1
>
> +# be sure to set LFS_CFLAGS if needed, required for libsmbclient on 32bits
> +CGO_CFLAGS := $(shell getconf LFS_CFLAGS)
> +export CGO_CFLAGS
> +
> %:
> dh $@ --buildsystem=golang --with=golang,apport

Would be interesting to know if 'export DEB_BUILD_MAINT_OPTIONS=future=+lfs'
also fixed the build, as conceptually that's the same level of abstraction
as fixing dpkg-buildflags per the above.

> I'm trying a rebuild of all reverse dependencies of libsmbclient to
> see if there are any other failures, but I am wondering if this is the
> current approach we want to take, or revert to our old-and-tried patch
> that just defines it for all apps including libsmbclient.h?

I think that if there's a solution that doesn't require patching upstream
samba, while guarding against miscompilation of either libraries or their
consumers and avoiding accidentally introducing any ABI changes, we should
prefer that.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer https://www.debian.org/
slangasek@ubuntu.com vorlon@debian.org