Re: [PATCHES] symbol visibility

[Marcel Holtmann <marcel holtmann org>]

Hi Dan,

> > while looking through the TODO list for 0.7 [1], I decided that I should 
> > contribute a bit, especially as it was me, that complained about the 
> > long list of exported symbols by libnm-glib and libnm-util.
> > 
> > So I went ahead and studied the docs a bit.
> > There are basically two approaches, how to trim down the list of 
> > exported symbols:
> > 
> > a) GCC's visibility support [2]
> >   GCC extension, mostly used by C++ libraries
> > 
> > b) ld linker/version scripts  [3]
> >   b.1) either directly via LDFLAGS -Wl,--version-script
> >   b.2) or indirectly via libtools -export-symbols $FILE or 
> > -export-symbols-regex, which will create a version script on the fly.
> > 
> > Before trying to evaluate which approach is best, I analyzed the list of 
> > exported symbols (see attached *.symbols files) and checked which 
> > symbols are not part of the public API (see attached *.diff files).
> > 
> > There are basically 5 ways
> > GCC visibility
> > 1.) Use -fvisibility=default (all symbols are exported by default) and 
> > augment all function declarations, which shall not be exported with 
> > __attribute__ ((visibility("hidden")))
> > 
> > Does not work, as we have autogenerated files, where we cannot add the 
> > visibility attribute.
> > 
> > 2.) Use -fvisibility=hidden (all symbols are hidden by default) and 
> > augment all function declarations, which shall be exported with 
> > __attribute__ ((visibility("default"))).
> > 
> > Lots of code changes in the header files. To be manageable we'd have to 
> > define a macro like DLLEXPORT. All installed header files would either 
> > have to define such a macro or we would have to create a new header file 
> > (like visibility.h) which contains this macro and all other header files 
> > include that. Imho quite cumbersome
> > 
> > libtool
> > 3.) -export-symbols-regex:
> > as you can see from the attached symbols and diff files, we can't use a 
> > common prefix like nm_.*. We'd have to rename private function to 
> > something like _nm_* -> looks of code changes.
> > 
> > 4.) -export-symbols $FILE
> > List all exported symbols in a separate file and pass it to libtool.
> > I chatted with libtool's upstream a bit, and he recommended though, to 
> > directly use a ld version script. as this provides the most flexibility,
> > 
> > 5.) LDFLAGS = -Wl,--version-script=$FILE
> > That's the way I implemented it in the patch. I listed each exported 
> > symbol explicitely (although the version script syntax would allow 
> > regexes). See patch 0001 and 0002 for the details.
> > Imho this is the nicest and cleanest approaches which requires the least 
> > changes.
> > I generated the version scripts automatically (by using objdump and 
> > grepping for the symbols in the installed header files, dropping all 
> > symbols which are not part of the public API), so a careful review would 
> > be very appreciated.
> > NOTE: I didn't enable symbol versioning as I didn't see the need for it 
> > yet, I only used the version script to control the list of exported symbols
> > 
> > After applying 0001 and 0002, the test-crypto binary will no longer 
> > build, as it uses private symbols from libnm-util. I simply removed it 
> > in patch 0003. An alternative would be, to add crypto.c and 
> > crypto_(gnutls|nss).c to test_crypto_SOURCES.
> > If someone has a need for this binary, I could change that.
> > 
> > 
> > Comments and feedback welcome.
> 
> I poked into visibility and talked to people who know stuff (ie Drepper)
> and the version script option seems to be most highly recommended one
> for a number of reasons.  So feel free to commit.

I ran into a similar problem with ConnMan a while ago and using libtool
for a binary doesn't seem to work. The versioning script is really the
best solution I could come up with. With some automake magic, I was able
to auto-generate the version file.

connmand_LDFLAGS = -Wl,--version-script=connman.ver

connmand_DEPENDENCIES = connman.ver

CLEANFILES = connman.ver connman.exp

connman.exp:
	nm -B *.o | awk '{ print $$3 }' | sort -u | grep -E -e '^connman_' > $@

connman.ver: connman.exp
	echo "{ global:" > $@
	cat $< | sed -e "s/\(.*\)/\1;/" >> $@
	echo "local: *; };" >> $@

So in this case, everything with the prefix connman_ will be made
public.

If the versioning script would handle regular expression by itself, then
that would be a lot simpler actually. At least I couldn't get that to
work.

Regards

Marcel