Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autotools: avoid passing LDFLAGS twice for libcurl #12310

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 13, 2023

autotools passes LDFLAGS automatically linker commands. curl's
lib/Makefile.am customizes libcurl linker flags. In that
customization, it added LDFLAGS to the custom flags. This resulted
in passing LDFLAGS twice to the libtool command.

Most of the time this is benign, but some LDFLAGS options can break
the build when passed twice. One such example is passing .o files,
e.g. crt*.o files necessary when customizing the C runtime, e.g. for
MUSL builds.

Passing them twice resulted in duplicate symbol errors:

libtool: link:  clang-15 --target=aarch64-unknown-linux-musl [...] /usr/lib/aarch64-linux-musl/crt1.o [...] /usr/lib/aarch64-linux-musl/crt1.o [...]
ld.lld-15: error: duplicate symbol: _start
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
[...]
clang: error: linker command failed with exit code 1 (use -v to see invocation)

This behaviour came with commit 1a59319
(2013-07-23) as a fix for bug https://curl.haxx.se/bug/view.cgi?id=1217.
The patch was a works-for-me hack that ended up merged in curl:
https://sourceforge.net/p/curl/bugs/1217/#06ef
With the root cause remaining unclear.

Perhaps the SUNPro 12 linker was sensitive to -L -l order, requiring
-L first? This would be unusual and suggests a bug in either the
linker or in libtool.

The curl build does pass the list of detected libs via its own
LIBCURL_LIBS variable, which ends up before LDFLAGS on the libtool
command line, but it's the job of libtool to ensure that even
a peculiar linker gets the options in the expected order. Also because
autotools passes LDFLAGS last, making it hardly possible to pass
anything after it.

Perhaps in the 10 years since this issue, this already got a fix
upstream.

This patch deletes LDFLAGS from our customized libcurl options,
leaving a single copy of them as passed by autotools automatically.

Reverts 1a59319
Closes #12310

@vszakats vszakats added the build label Nov 13, 2023
@vszakats vszakats changed the title autotools: avoid passing LDFLAGS twice to libcurl autotools: avoid passing LDFLAGS twice for libcurl Nov 13, 2023
autotools passes `LDFLAGS` automatically linker commands. curl's
`lib/Makefile.am` customizes libcurl linker flags. In that
customization, it added `LDFLAGS` to the custom flags. This resulted in
passing `LDFLAGS` _twice_ to the `libtool` command.

Most of the time this is benign, but some `LDFLAGS` options can break
the build when passed twice. One such example is passing `.o` files,
e.g. `crt*.o` files necessary when customizing the C runtime, e.g. for
MUSL builds.

Passing them twice resulted in duplicate symbol errors:
```
libtool: link:  clang-15 --target=aarch64-unknown-linux-musl [...] /usr/lib/aarch64-linux-musl/crt1.o [...] /usr/lib/aarch64-linux-musl/crt1.o [...]
ld.lld-15: error: duplicate symbol: _start
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
[...]
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

This behaviour came with commit 1a59319
(2013-07-23) as a fix for bug https://curl.haxx.se/bug/view.cgi?id=1217.
The patch was a works-for-me hack that ended up merged in curl:
https://sourceforge.net/p/curl/bugs/1217/#06ef
With the root cause remaining unclear.

Perhaps the SUNPro 12 linker was sensitive to `-L` `-l` order, requiring
`-L` first? This would be unusual and suggests a bug in either the
linker or in `libtool`.

The curl build does pass the list of detected libs via its own
`LIBCURL_LIBS` variable, which ends up before `LDFLAGS` on the `libtool`
command line, but it's the job of `libtool` to ensure that even
a peculiar linker gets the options in the expected order. Also because
autotools passes `LDFLAGS` last, making it hardly possible to pass
anything after it.

Perhaps in the 10 years since this issue, this already got a fix
upstream.

This patch deletes `LDFLAGS` from our customized libcurl options,
leaving a single copy of them as passed by autotools automatically.

Reverts 1a59319
Closes curl#12310
@vszakats vszakats closed this in 444f64b Nov 14, 2023
@vszakats vszakats deleted the autotools-fix-double-ldflags branch November 14, 2023 19:29
zuoxiaofeng pushed a commit to zuoxiaofeng/curl that referenced this pull request Nov 28, 2023
autotools passes `LDFLAGS` automatically linker commands. curl's
`lib/Makefile.am` customizes libcurl linker flags. In that
customization, it added `LDFLAGS` to the custom flags. This resulted in
passing `LDFLAGS` _twice_ to the `libtool` command.

Most of the time this is benign, but some `LDFLAGS` options can break
the build when passed twice. One such example is passing `.o` files,
e.g. `crt*.o` files necessary when customizing the C runtime, e.g. for
MUSL builds.

Passing them twice resulted in duplicate symbol errors:
```
libtool: link:  clang-15 --target=aarch64-unknown-linux-musl [...] /usr/lib/aarch64-linux-musl/crt1.o [...] /usr/lib/aarch64-linux-musl/crt1.o [...]
ld.lld-15: error: duplicate symbol: _start
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
>>> defined at crt1.c
>>>            /usr/lib/aarch64-linux-musl/crt1.o:(.text+0x0)
[...]
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```

This behaviour came with commit 1a59319
(2013-07-23) as a fix for bug https://curl.haxx.se/bug/view.cgi?id=1217.
The patch was a works-for-me hack that ended up merged in curl:
https://sourceforge.net/p/curl/bugs/1217/#06ef
With the root cause remaining unclear.

Perhaps the SUNPro 12 linker was sensitive to `-L` `-l` order, requiring
`-L` first? This would be unusual and suggests a bug in either the
linker or in `libtool`.

The curl build does pass the list of detected libs via its own
`LIBCURL_LIBS` variable, which ends up before `LDFLAGS` on the `libtool`
command line, but it's the job of `libtool` to ensure that even
a peculiar linker gets the options in the expected order. Also because
autotools passes `LDFLAGS` last, making it hardly possible to pass
anything after it.

Perhaps in the 10 years since this issue, this already got a fix
upstream.

This patch deletes `LDFLAGS` from our customized libcurl options,
leaving a single copy of them as passed by autotools automatically.

Reverts 1a59319
Closes curl#12310
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

1 participant