[Mesa-dev] [PATCH] util/disk_cache: fix zlib linking with LTO build

[Emil Velikov]

On 10 March 2017 at 13:36, Steven Newbury <steve at snewbury.org.uk> wrote:
> On Fri, 2017-03-10 at 12:11 +0000, Emil Velikov wrote:
>> On 9 March 2017 at 14:39, Steven Newbury <steve at snewbury.org.uk>
>> wrote:
>> > Introduction of zlib compression for the shader cache means
>> > zlib needs to be explicitly linked to libOSMesa and libstandalone
>> > otherwise build fails when LTO is used.
>> > ---
>>
>> How exactly are you doing the LTO build ?
> I build everything LTO, except for a short blacklist where
> unsupportable.  Specifically, on this system my global *FLAGS contain
> "-flto=8 -fuse-linker-plugin".  I have the lto linker plugin symlinked
> into "/usr/$CHOST/binutils-bin/lib/bfd-plugins/".
>
> I also have have the following env vars set to ensure they are called
> with the LTO plugin:
> AR="gcc-ar"
> NM="gcc-nm"
> RANLIB="gcc-ranlib"
>
One should not need these. Have you checked that the issue is
reproducible w/o them ?

> Mesa is not blacklisted, and with the patch, and previous to the
> compressed shader cache built fine, but otherwise there are undefined
> zlib symbols.
>>
>> The only user of ZLIB is libmesautil.la which in itself uses
>> ZLIB_LIBS.
> It does, and that seems to satisfy linking of libmesautil.la, but it
> doesn't result in the archive containing a static zlib since you're
> using "-lz" (at least with the output of pkg-config --libs zlib here)
> not explicitly linking to libz.a.  (My understanding.)
>
Can you compare (list here) the link line (make V=1) for the affected
objects with and w/o LTO ?
There should be -lz in the former case, at least.

>> Hence as-is patch looks wrong/incomplete, although I might be missing
>> something ?
> I guess that depends upon your point of view.  To me it's the preferred
> behaviour to use the system provided dynamic zlib to resolve the
> symbols at load time.
We have multiple places that use libmesautil and adding zlib to only
some _is_ incomplete.
Not to mention that in the OSMesa case - there's nothing in there that
uses libmesautil or zlib directly or not, afaict. Hence 'being wrong'.

If there's a bug somewhere, hence the need for ZLIB_LIBS, that should
be clearly documented alongside the patch.

Thanks
Emil