Don't combine '-r' and '--export-dynamic' linker options

Message ID 20191025012013.118919-1-espindola@scylladb.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series Don't combine '-r' and '--export-dynamic' linker options |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Rafael Ávila de Espíndola Oct. 25, 2019, 1:20 a.m. UTC
  Running ld with '-r' switches the linker to a very special mode where
some other linker options don't make sense.

In particular, '--export-dynamic' normally requires that all global
symbols be included in the dynamic symbol table, but a .o file doesn't
even have a dynamic symbol table.

When given both options it looks like the gnu linker just ignores
'--export-dynamic'.

Unfortunately some versions of lld (https://lld.llvm.org/) have a bug
that causes it to try to create a dynamic symbol table in the output
.o file and ends up corrupting it
(https://bugs.llvm.org/show_bug.cgi?id=43552). Current (git) version
of lld now issues an error.

This patch drops $(LDFLAGS) when using -r. With this patch I can build
dpdk with lld.

Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
---
 mk/internal/rte.compile-pre.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Rafael Ávila de Espíndola Dec. 6, 2019, 10:51 p.m. UTC | #1
Hi Thomas,

Are you the right maintainer fer reviewing this patch?

Thanks,
Rafael

Rafael Ávila de Espíndola <espindola@scylladb.com> writes:

> Running ld with '-r' switches the linker to a very special mode where
> some other linker options don't make sense.
>
> In particular, '--export-dynamic' normally requires that all global
> symbols be included in the dynamic symbol table, but a .o file doesn't
> even have a dynamic symbol table.
>
> When given both options it looks like the gnu linker just ignores
> '--export-dynamic'.
>
> Unfortunately some versions of lld (https://lld.llvm.org/) have a bug
> that causes it to try to create a dynamic symbol table in the output
> .o file and ends up corrupting it
> (https://bugs.llvm.org/show_bug.cgi?id=43552). Current (git) version
> of lld now issues an error.
>
> This patch drops $(LDFLAGS) when using -r. With this patch I can build
> dpdk with lld.
>
> Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
> ---
>  mk/internal/rte.compile-pre.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
> index 0cf3791b4..6cd7e559a 100644
> --- a/mk/internal/rte.compile-pre.mk
> +++ b/mk/internal/rte.compile-pre.mk
> @@ -61,7 +61,7 @@ CHECK_EXPERIMENTAL = $(EXPERIMENTAL_CHECK) $(SRCDIR)/$(EXPORT_MAP) $@
>  
>  PMDINFO_GEN = $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c
>  PMDINFO_CC = $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@.pmd.o $@.pmd.c
> -PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
> +PMDINFO_LD = $(CROSS)ld -r -o $@.o $@.pmd.o $@
>  PMDINFO_TO_O = if grep -q 'RTE_PMD_REGISTER_.*(.*)' $<; then \
>  	echo "$(if $V,$(PMDINFO_GEN),  PMDINFO $@.pmd.c)" && \
>  	$(PMDINFO_GEN) && \
> -- 
> 2.21.0
  
Thomas Monjalon Dec. 12, 2019, 4:05 p.m. UTC | #2
25/10/2019 03:20, Rafael Ávila de Espíndola:
> Running ld with '-r' switches the linker to a very special mode where
> some other linker options don't make sense.
> 
> In particular, '--export-dynamic' normally requires that all global
> symbols be included in the dynamic symbol table, but a .o file doesn't
> even have a dynamic symbol table.
> 
> When given both options it looks like the gnu linker just ignores
> '--export-dynamic'.
> 
> Unfortunately some versions of lld (https://lld.llvm.org/) have a bug
> that causes it to try to create a dynamic symbol table in the output
> .o file and ends up corrupting it
> (https://bugs.llvm.org/show_bug.cgi?id=43552). Current (git) version
> of lld now issues an error.
> 
> This patch drops $(LDFLAGS) when using -r. With this patch I can build
> dpdk with lld.
[...]
> -PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
> +PMDINFO_LD = $(CROSS)ld -r -o $@.o $@.pmd.o $@

Dealing with compiler bugs is really annoying.
I'm afraid removing LDFLAGS may break in some environments.
Could you just filter-out some incompatible options?

And what about meson? Is there some similar issue?
  
Rafael Ávila de Espíndola Dec. 12, 2019, 5:44 p.m. UTC | #3
Thomas Monjalon <thomas@monjalon.net> writes:

> 25/10/2019 03:20, Rafael Ávila de Espíndola:
>> Running ld with '-r' switches the linker to a very special mode where
>> some other linker options don't make sense.
>> 
>> In particular, '--export-dynamic' normally requires that all global
>> symbols be included in the dynamic symbol table, but a .o file doesn't
>> even have a dynamic symbol table.
>> 
>> When given both options it looks like the gnu linker just ignores
>> '--export-dynamic'.
>> 
>> Unfortunately some versions of lld (https://lld.llvm.org/) have a bug
>> that causes it to try to create a dynamic symbol table in the output
>> .o file and ends up corrupting it
>> (https://bugs.llvm.org/show_bug.cgi?id=43552). Current (git) version
>> of lld now issues an error.
>> 
>> This patch drops $(LDFLAGS) when using -r. With this patch I can build
>> dpdk with lld.
> [...]
>> -PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
>> +PMDINFO_LD = $(CROSS)ld -r -o $@.o $@.pmd.o $@
>
> Dealing with compiler bugs is really annoying.
> I'm afraid removing LDFLAGS may break in some environments.
> Could you just filter-out some incompatible options?

Sure, that works. I will send a v2.

> And what about meson? Is there some similar issue?

As far as I can tell meson produces a .a file instead. In the
build.ninja file I see

build drivers/librte_pmd_dpaa_event.a: STATIC_LINKER drivers/a715181@@rte_pmd_dpaa_event@sta/meson-generated_.._rte_pmd_dpaa_event.pmd.c.o drivers/a715181@@tmp_rte_pmd_dpaa_event@sta/event_dpaa_dpaa_eventdev.c.o

and

rule STATIC_LINKER
 command = rm -f $out && gcc-ar $LINK_ARGS $out $in

Cheers,
Rafael
  
Ray Kinsella Dec. 13, 2019, 10:07 a.m. UTC | #4
On 12/12/2019 16:05, Thomas Monjalon wrote:
> 25/10/2019 03:20, Rafael Ávila de Espíndola:
>> Running ld with '-r' switches the linker to a very special mode where
>> some other linker options don't make sense.
>>
>> In particular, '--export-dynamic' normally requires that all global
>> symbols be included in the dynamic symbol table, but a .o file doesn't
>> even have a dynamic symbol table.
>>
>> When given both options it looks like the gnu linker just ignores
>> '--export-dynamic'.
>>
>> Unfortunately some versions of lld (https://lld.llvm.org/) have a bug
>> that causes it to try to create a dynamic symbol table in the output
>> .o file and ends up corrupting it
>> (https://bugs.llvm.org/show_bug.cgi?id=43552). Current (git) version
>> of lld now issues an error.
>>
>> This patch drops $(LDFLAGS) when using -r. With this patch I can build
>> dpdk with lld.
> [...]
>> -PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
>> +PMDINFO_LD = $(CROSS)ld -r -o $@.o $@.pmd.o $@
> 
> Dealing with compiler bugs is really annoying.
> I'm afraid removing LDFLAGS may break in some environments.
> Could you just filter-out some incompatible options?

Right, detect that LLD is the linker and change LDFLAGS accordingly.
Painful, but the right approach.

> 
> And what about meson? Is there some similar issue?
> 
> 
>
  

Patch

diff --git a/mk/internal/rte.compile-pre.mk b/mk/internal/rte.compile-pre.mk
index 0cf3791b4..6cd7e559a 100644
--- a/mk/internal/rte.compile-pre.mk
+++ b/mk/internal/rte.compile-pre.mk
@@ -61,7 +61,7 @@  CHECK_EXPERIMENTAL = $(EXPERIMENTAL_CHECK) $(SRCDIR)/$(EXPORT_MAP) $@
 
 PMDINFO_GEN = $(RTE_SDK_BIN)/app/dpdk-pmdinfogen $@ $@.pmd.c
 PMDINFO_CC = $(CC) $(CPPFLAGS) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@.pmd.o $@.pmd.c
-PMDINFO_LD = $(CROSS)ld $(LDFLAGS) -r -o $@.o $@.pmd.o $@
+PMDINFO_LD = $(CROSS)ld -r -o $@.o $@.pmd.o $@
 PMDINFO_TO_O = if grep -q 'RTE_PMD_REGISTER_.*(.*)' $<; then \
 	echo "$(if $V,$(PMDINFO_GEN),  PMDINFO $@.pmd.c)" && \
 	$(PMDINFO_GEN) && \