Android: Fix LLVM duplicated symbols linking for N and M

Submitted by Rob Herring on Aug. 21, 2017, 4:17 p.m.

Details

Message ID CAL_JsqJD+4eK2+-=Y6tmr7O_LLNFw+sz4ry4Z0KLhGf=vdAxSQ@mail.gmail.com
State New
Headers show
Series "Android: Fix LLVM duplicated symbols linking for N and M" ( rev: 2 ) in Mesa

Browsing this patch as part of:
"Android: Fix LLVM duplicated symbols linking for N and M" rev 2 in Mesa
<< prev patch [1/1] next patch >>

Commit Message

Rob Herring Aug. 21, 2017, 4:17 p.m.
On Fri, Aug 18, 2017 at 7:27 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 18 August 2017 at 20:46, Rob Herring <robh@kernel.org> wrote:
>> Both statically linking libLLVMCore and dynamically linking libLLVM causes
>> duplicated symbols in gallium_dri.so and it fails to dlopen. We don't
>> really need to link libLLVMCore, but just need generated headers to be
>> built first. Dynamically linking to libLLVM instead is enough to do
>> that. Thanks to Qiang Yu for finding the root cause.
>>
> Nice find indeed, thanks.
>
> This reminds me - a small task for a rainy day.
> - Wire the version script files into the Android build - see the
> autoconf snippet below.
> It will hide the hundreds of symbols when static linking LLVM (aka
> sidestep the current issue) and make the binary noticeably smaller.
>
> gallium_dri_la_LDFLAGS += \
>        -Wl,--version-script=$(top_srcdir)/src/gallium/targets/dri/dri.sym

About 60K smaller out of 12MB. Is that inline with what you'd expect?

before:
   text    data     bss     dec     hex filename
12164054         420384  352476 12936914         c566d2
out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so

after:
   text    data     bss     dec     hex filename
12104616         419816  352476 12876908         c47c6c
out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so


The change is trivial, though I'm not sure if there's any downside to
setting --undefined-version (The Android build system turns this off).

Patch hide | download patch | download mbox

diff --git a/src/gallium/targets/dri/Android.mk
b/src/gallium/targets/dri/Android.mk
index 150b2e368e51..440dded0cebe 100644
--- a/src/gallium/targets/dri/Android.mk
+++ b/src/gallium/targets/dri/Android.mk
@@ -32,6 +32,8 @@  LOCAL_SRC_FILES := target.c

 LOCAL_CFLAGS :=

+LOCAL_LDFLAGS := -Wl,--version-script=$(LOCAL_PATH)/dri.sym
-Wl,--undefined-version
+
 LOCAL_SHARED_LIBRARIES := \
        libdl \
        liblog \

Comments

On 21 August 2017 at 17:17, Rob Herring <robh@kernel.org> wrote:
> On Fri, Aug 18, 2017 at 7:27 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> On 18 August 2017 at 20:46, Rob Herring <robh@kernel.org> wrote:
>>> Both statically linking libLLVMCore and dynamically linking libLLVM causes
>>> duplicated symbols in gallium_dri.so and it fails to dlopen. We don't
>>> really need to link libLLVMCore, but just need generated headers to be
>>> built first. Dynamically linking to libLLVM instead is enough to do
>>> that. Thanks to Qiang Yu for finding the root cause.
>>>
>> Nice find indeed, thanks.
>>
>> This reminds me - a small task for a rainy day.
>> - Wire the version script files into the Android build - see the
>> autoconf snippet below.
>> It will hide the hundreds of symbols when static linking LLVM (aka
>> sidestep the current issue) and make the binary noticeably smaller.
>>
>> gallium_dri_la_LDFLAGS += \
>>        -Wl,--version-script=$(top_srcdir)/src/gallium/targets/dri/dri.sym
>
> About 60K smaller out of 12MB. Is that inline with what you'd expect?
>
Sounds a bit smaller than expected, but every little helps ;-)

> before:
>    text    data     bss     dec     hex filename
> 12164054         420384  352476 12936914         c566d2
> out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so
>
> after:
>    text    data     bss     dec     hex filename
> 12104616         419816  352476 12876908         c47c6c
> out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so
>
>
> The change is trivial, though I'm not sure if there's any downside to
> setting --undefined-version (The Android build system turns this off).
>
There's no actual symbol versioning, just restriction of the actual
exported symbols.
Thus things should work w/o -Wl,--undefined-version so I wouldn't touch it.

-Emil
On Mon, Aug 21, 2017 at 12:47 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 21 August 2017 at 17:17, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Aug 18, 2017 at 7:27 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> On 18 August 2017 at 20:46, Rob Herring <robh@kernel.org> wrote:
>>>> Both statically linking libLLVMCore and dynamically linking libLLVM causes
>>>> duplicated symbols in gallium_dri.so and it fails to dlopen. We don't
>>>> really need to link libLLVMCore, but just need generated headers to be
>>>> built first. Dynamically linking to libLLVM instead is enough to do
>>>> that. Thanks to Qiang Yu for finding the root cause.
>>>>
>>> Nice find indeed, thanks.
>>>
>>> This reminds me - a small task for a rainy day.
>>> - Wire the version script files into the Android build - see the
>>> autoconf snippet below.
>>> It will hide the hundreds of symbols when static linking LLVM (aka
>>> sidestep the current issue) and make the binary noticeably smaller.
>>>
>>> gallium_dri_la_LDFLAGS += \
>>>        -Wl,--version-script=$(top_srcdir)/src/gallium/targets/dri/dri.sym
>>
>> About 60K smaller out of 12MB. Is that inline with what you'd expect?
>>
> Sounds a bit smaller than expected, but every little helps ;-)
>
>> before:
>>    text    data     bss     dec     hex filename
>> 12164054         420384  352476 12936914         c566d2
>> out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so
>>
>> after:
>>    text    data     bss     dec     hex filename
>> 12104616         419816  352476 12876908         c47c6c
>> out/target/product/linaro_x86_64/system/lib64/dri/gallium_dri.so
>>
>>
>> The change is trivial, though I'm not sure if there's any downside to
>> setting --undefined-version (The Android build system turns this off).
>>
> There's no actual symbol versioning, just restriction of the actual
> exported symbols.
> Thus things should work w/o -Wl,--undefined-version so I wouldn't touch it.

No it doesn't, because it can't find __driDriverExtensions.

Rob