Page MenuHomeFreeBSD
Differential D11504

Provide libdl.
ClosedPublic
Actions

Authored by kib on Jul 6 2017, 5:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 10:14 PM
Unknown Object (File)
Thu, Dec 5, 4:34 PM
Unknown Object (File)
Tue, Dec 3, 3:57 AM
Unknown Object (File)
Sat, Nov 30, 5:38 PM
Unknown Object (File)
Sat, Nov 30, 5:38 PM
Unknown Object (File)
Sat, Nov 30, 5:33 PM
Unknown Object (File)
Sat, Nov 30, 5:33 PM
Unknown Object (File)
Sat, Nov 30, 5:33 PM
View All Files
Subscribers
bapt
bdrewery
imp
mat
mmokhi

Details

Reviewers
emaste
dim
bdrewery
Group Reviewers
portmgr
Commits
rS320872: Provide libdl.
Summary

After 20+ years, we must admit a defeat and agree that the external world wants -ldl to work.

Create libdl.so.1 as a filter for lbc.so.7 which exports public dl* functions. The functions are resolved from the rtld instead, the goal of creating library is to avoid errors from the static linker due to missed libdl. For static binaries, I compile empty .o into libdl.a so that static binaries still get dl stubs from libc.a.

This should reduce the need to patch out -ldl from naive builds, and reduce the need to fix errors regarding detection of -ldl from less naive. I do not expect that libdl.so will be used by base system.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib created this revision.Jul 6 2017, 5:52 AM
Herald added subscribers: imp, bdrewery. · View Herald TranscriptJul 6 2017, 5:52 AM
mat added a subscriber: mat.Jul 6 2017, 7:41 AM
mat added inline comments.
lib/libc/gen/dlfcn.c
70 ↗(On Diff #30480)

any reason to add all those empty lines ?

kib added inline comments.Jul 6 2017, 8:48 AM
lib/libc/gen/dlfcn.c
70 ↗(On Diff #30480)

Style(9) requires that the declaration block and the function body were separated by an empty line, even if the declarations are empty.

bapt added a subscriber: bapt.Jul 6 2017, 8:59 AM

I'm puzzled about this proposal. Lots of tools except us not to provide libdl, so I think adding this will make us have to patch probably things like libtool, autotools, cmake etc. Also when other BSD's are patching upstream build systems they usually add a if BSD to cover all of the BSDs. Another issue migth be to have to patch the ports tree so if freebsd 12 then -ldl else nothing.

That said, Illumos is providing a libdl and as far as I know the BSD family is the last one without a libdl.

kib added a comment.Jul 6 2017, 9:18 AM
In D11504#237900, @bapt wrote:

I'm puzzled about this proposal. Lots of tools except us not to provide libdl, so I think adding this will make us have to patch probably things like libtool, autotools, cmake etc. Also when other BSD's are patching upstream build systems they usually add a if BSD to cover all of the BSDs. Another issue migth be to have to patch the ports tree so if freebsd 12 then -ldl else nothing.

That said, Illumos is providing a libdl and as far as I know the BSD family is the last one without a libdl.

Well. You are not required to use -ldl. Not providing -ldl after the patch still work same as now, but also it works to link with it. In other words, I expect some patching only for stuff which is getting confused by expectation to not see libdl but finding it.

Similarly, for FreeBSD 12, you do not need forcibly add -ldl.

But people that get a code which unconditionally add -ldl, the things start work out of the box.

bapt added a comment.Jul 6 2017, 9:21 AM

A sorry I haven't looked close enough to the patch. then I have no strong objection against it if both works.

mmokhi added a subscriber: mmokhi.Jul 6 2017, 10:15 AM
dim accepted this revision.Jul 6 2017, 5:34 PM

I'm fine with this. It might save some end-user hassle and/or confusion, and it seems relatively easy to maintain this wrapper library.

This revision is now accepted and ready to land.Jul 6 2017, 5:34 PM
bdrewery requested changes to this revision.Jul 6 2017, 5:54 PM
bdrewery added inline comments.
share/mk/bsd.libnames.mk
59 ↗(On Diff #30480)

This won't be needed if the base system is not using it. If we think this is needed we should add it to the _LIBRARIES list in src.libnames.mk as well for LIBADD.

This revision now requires changes to proceed.Jul 6 2017, 5:54 PM
emaste edited edge metadata.Jul 6 2017, 5:59 PM

I think we ought to have some note in dlopen.2 mentioning that libdl is provided for compatibility on FreeBSD but is not needed.

lib/libc/gen/dlfcn.c
70 ↗(On Diff #30480)

I think style(9) is broken in this regard :)

kib updated this revision to Diff 30503.Jul 6 2017, 6:34 PM
kib edited edge metadata.
kib marked an inline comment as done.

Add libdl to src.libnames.mk and mdoc' lib.in.
Mention its existence in dlopen(3).

share/mk/bsd.libnames.mk
59 ↗(On Diff #30480)

This is not needed now, but I do not want to prevent use of -ldl with the native builds.

bdrewery accepted this revision as: bdrewery.Jul 6 2017, 6:44 PM
emaste accepted this revision.Jul 6 2017, 6:50 PM
emaste added inline comments.
lib/libc/gen/dlopen.3
382 ↗(On Diff #30503)

Perhaps "to provide .Fn dlopen and other functions."

kib updated this revision to Diff 30505.Jul 6 2017, 6:57 PM
kib edited edge metadata.

Fix language.

emaste accepted this revision.Jul 6 2017, 7:01 PM
kib added a comment.Jul 6 2017, 7:07 PM

Exp run PR 220525.

kib mentioned this in rS320868: Fix warnings, adjust style..Jul 10 2017, 8:11 AM
kib updated this revision to Diff 30623.Jul 10 2017, 12:51 PM
kib edited edge metadata.

lld cannot create filter objects, disable libdl on arm64.

emaste added inline comments.Jul 10 2017, 1:46 PM
share/mk/bsd.linker.mk
73 ↗(On Diff #30623)

I suppose we'll need to add detection for gold as well

kib added inline comments.Jul 10 2017, 2:14 PM
share/mk/bsd.linker.mk
73 ↗(On Diff #30623)

This is exactly the reason why I asked for features and to avoid using linker type directly.

That said, I was very surprised that lld does not support such trivial (for a static linker) feature as filters. Note that libsupc++ requires filters, so I am not sure how much useful lld is even on arm64.

emaste accepted this revision.Jul 10 2017, 2:34 PM
emaste added inline comments.
share/mk/bsd.linker.mk
73 ↗(On Diff #30623)

Indeed. That said, LLD has implemented every feature we require in short order, so I expect that we'll be able to revert this part and assume all linkers support --filter in the near future.

Closed by commit rS320872: Provide libdl. (authored by kib). · Explain WhyJul 10 2017, 2:59 PM
This revision was automatically updated to reflect the committed changes.
emaste added a comment.Jul 13 2017, 1:49 PM

Filter support is in review at https://reviews.llvm.org/D35352.

kib added a comment.Jul 13 2017, 2:05 PM
In D11504#239687, @emaste wrote:

Filter support is in review at https://reviews.llvm.org/D35352.

What about aux filters ? The patch should be same.

emaste added a comment.Jul 13 2017, 2:09 PM

What about aux filters ? The patch should be same.

Somewhat surprisingly aux filters are already implemented. One interesting note from the LLD work: it seems GNU ld accepts multiple aux filter args, but only a single std filter. I think this is a GNU ld bug.

kib added a comment.Jul 13 2017, 2:13 PM
In D11504#239695, @emaste wrote:

Somewhat surprisingly aux filters are already implemented. One interesting note from the LLD work: it seems GNU ld accepts multiple aux filter args, but only a single std filter. I think this is a GNU ld bug.

Most likely this explains why libsupc++ builds.

Revision Contents
Changeset List

PathSize
head/
contrib/
mdocml/
1 line
lib/
5 lines
libc/
gen/
10 lines
10 lines
libdl/
15 lines
20 lines
share/
mk/
1 line
3 lines
1 line

Diff 30625

View Options

head/contrib/mdocml/lib.in

Loading...
View Options

head/lib/Makefile

Loading...
View Options

head/lib/libc/gen/dlfcn.c

Loading...
View Options

head/lib/libc/gen/dlopen.3

Loading...
View Options

head/lib/libdl/Makefile

Loading...
View Options

head/lib/libdl/Symbol.map

Loading...
View Options

head/share/mk/bsd.libnames.mk

Loading...
View Options

head/share/mk/bsd.linker.mk

Loading...
View Options

head/share/mk/src.libnames.mk

Loading...