Date: Fri, 16 May 2025 20:18:29 +0300 From: Andriy Gapon <avg@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, FreeBSD Current <current@freebsd.org> Subject: libdtrace symbol map [Was: RTLD_DEEPBIND question] Message-ID: <ade2d044-95be-4fcf-857b-b6435aba7bb8@freebsd.org> In-Reply-To: <aCdHgFCQA1ghPfYJ@nuc> References: <aALjHwFnFKChuAdR@kib.kiev.ua> <900c8521-559a-47b5-acaa-ae941f6852c4@freebsd.org> <fd12cce4-7e6b-4ab6-bced-b36e98c995ba@FreeBSD.org> <7c4e1682-d797-493c-8326-08d51dde3359@FreeBSD.org> <aAN69URjEJFuOLxR@kib.kiev.ua> <aAN9NH1IR-gZceP4@kib.kiev.ua> <e528f630-9d6e-4ec6-b7e6-30b5a978f5c8@freebsd.org> <59db8ace-770f-4f73-976f-411f6de0885a@FreeBSD.org> <aA4-hQ6xrF2dD-sJ@nuc> <5b887290-a6aa-49ff-aa92-f335ed09b9a5@freebsd.org> <aCdHgFCQA1ghPfYJ@nuc>
index | next in thread | previous in thread | raw e-mail
On 16/05/2025 17:11, Mark Johnston wrote: > On Fri, May 16, 2025 at 03:22:52PM +0300, Andriy Gapon wrote: >> On 27/04/2025 17:26, Mark Johnston wrote: >>> On Thu, Apr 24, 2025 at 08:56:44AM +0300, Andriy Gapon wrote: >>>> On 23/04/2025 21:56, Andriy Gapon wrote: >>>>> BTW, I've been wondering how illumos avoids the problem even though they >>>>> do not use any special dlopen flags. >>>>> It turns out that they link almost all system shared libraries with >>>>> -Bdirect option (which is Solaris/illumos specific). >>>>> It's somewhat similar to, but different from, -Bsymbolic. >>>>> https://docs.oracle.com/cd/E23824_01/html/819-0690/aehzq.html#scrolltoc >>>>> https://docs.oracle.com/cd/E36784_01/html/E36857/gejfe.html >>>> >>>> Oh, and it looks like there is an even better explanation for illumos. >>>> There is a version map file for libdtrace which explicitly lists API >>>> functions and makes everything else local. >>>> https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libdtrace/common/mapfile-vers >>>> >>>> I wonder why we didn't do the same when porting. >>>> Maybe we should do that now? >>> >>> I don't have any objection, but I believe adding a version map after the >>> fact doesn't accomplish much, assuming that we care deeply about ABI >>> stability in libdtrace.so (I'm not sure we do though). >> >> My primary goal here is not ABI stability, but hiding symbols that really >> should not be exported. See more at the end. >> >> At the same time I am not sure why it could be too late to start caring >> about ABI stability now. Assuming we actually would want to do that. > > I just mean that the version map helps only helps provide stability for > binaries linked to libdtrace.so after the version map is introduced. So, if we introduce it (and bump the library version as kib pointed out), then eventually it becomes useful (like it did for all other libraries that we versioned). >> And I don't want to single out libtrace here. >> It seems that the story is the same for all libraries that have been ported >> from illumos. >> E.g., libzfs_core was supposed to be a library that cares greatly about its >> API / ABI stability. >> >>>>> I think that on FreeBSD we should use symbol visibility attributes or a >>>>> symbol map to hide (make local) symbols that are not expected to be >>>>> interposed or have a high chance to be interposed by accident. >>>>> >>>>> IMO, yyparse should definitely get that treatment. >>>>> >>>>> I think that approach would be better than magic rtld tricks. >>>>> Especially because the tricks do not work with the current rtld. >>>>> I'd rather make a change to libdtrace.so than to rtld. >>>> >>>> This, while not as nice as the illumos solution, fixes my specific issue: >>>> diff --git a/cddl/lib/libdtrace/Makefile b/cddl/lib/libdtrace/Makefile >>>> index d086fffb07bc..58054d129b49 100644 >>>> --- a/cddl/lib/libdtrace/Makefile >>>> +++ b/cddl/lib/libdtrace/Makefile >>>> @@ -146,7 +146,8 @@ CFLAGS+= -fsanitize=address -fsanitize=undefined >>>> LDFLAGS+= -fsanitize=address -fsanitize=undefined >>>> .endif >>>> >>>> -LIBADD= ctf elf proc pthread rtld_db xo >>>> +VERSION_MAP= ${.CURDIR}/Symbol.map >>>> +LIBADD= ctf elf proc pthread rtld_db xo >>>> >>>> CLEANFILES= dt_errtags.c dt_names.c >>>> >>>> diff --git a/cddl/lib/libdtrace/Symbol.map b/cddl/lib/libdtrace/Symbol.map >>>> new file mode 100644 >>>> index 000000000000..89ee9de65209 >>>> --- /dev/null >>>> +++ b/cddl/lib/libdtrace/Symbol.map >>>> @@ -0,0 +1,4 @@ >>>> +{ >>>> + local: >>>> + yy*; >>>> +}; >>> >>> This just gives the lexer/parser symbols in libdtrace.so local >>> visibility? I think that's fine. >> Yes, that's the intention. >> >> I tested locally two versions of Symbol.map for libdtrace. >> The basic one quoted here and a more extended one based on illumos >> lib/libdtrace/common/mapfile-vers. >> The latter version does not define any symbol versions, its purpose is only >> to be a white-list of things to make public / global: >> https://people.freebsd.org/~avg/libdtrace-Symbol.map > > Do we really want to export _dtrace_debug? Hmm, I don't know, that came from illumos. However, I do not see any references to _dtrace_debug outside of libdtrace neither in FreeBSD nor in illumos. So, I guess that it can be removed. Maybe it was exposed for potential libdtrace consumers that might want to enable debug directly rather than via environment. >> Comparing to illumos I only had to add 3 dtrace_oformat* symbols, >> >> Both versions worked equally well in my tests, but maybe I missed more of >> FreeBSD extensions. >> >> Which one would be better to get into the tree? > > Having a full whitelist seems preferable to me. Did you test lockstat > as well? I believe it and dtrace(8) are the only users of libdtrace.so > in the base system. I didn't before, I've just done now, it works. I'll post a review request over the weekend (hopefully). LD_PRELOAD=/usr/obj/amd64.amd64/cddl/lib/libdtrace/libdtrace.so.2 lockstat -H -D 10 -x aggsize=100m -l smp_ipi_mtx -s 20 -P sleep 5 Spin lock hold: 106 events in 5.021 seconds (21 events/sec) ------------------------------------------------------------------------------- Count indv cuml rcnt nsec Lock Caller 11 53% 53% 0.00 99541 smp rendezvous __mtx_unlock_spin_flags+0xe3 nsec ------ Time Distribution ------ count Stack 16384 |@@ 1 smp_rendezvous_cpus+0x187 32768 |@@@@@@@@@@@@@@@@@@@ 7 dtrace_sync+0x39 65536 | 0 dtrace_state_deadman+0x13 131072 | 0 softclock_call_cc+0x1d9 262144 |@@ 1 softclock_thread+0xc6 524288 |@@@@@ 2 fork_exit+0x82 0xffffffff81030a3e ------------------------------------------------------------------------------- ... -- Andriy Gaponhome | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ade2d044-95be-4fcf-857b-b6435aba7bb8>