[PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[Ian Campbell]

... and limit the exported symbols to just those which are used in
tree.

This was partly just an interest driven excercise to see how much
internal cruft these libraries were actually exporting (it's not
actually as bad as I expected).

According to diffing before and after:
    nm libfoo.so | awk '{ if ($2 == "T") { print $3 } }'

The following symbols are no longer exported:

- libxenstore.so:
  - _fini
  - _init
- libxenctrl.so:
  - _fini
  - _init
  - bitmap_64_to_byte
  - bitmap_byte_to_64
  - discard_file_cache
- libxenguest.so:
  - _fini
  - _init
  - cr3_to_mfn
  - csum_page
  - dhdr_type_to_str
  - dump_bad_pseudophysmap_entry
  - handle_tsc_info
  - lz4_decompress_unknownoutputsize
  - mfn_in_pseudophysmap
  - mfn_to_cr3
  - mfn_to_pfn
  - pin_table
  - populate_pfns
  - rec_type_to_str
  - write_split_record
  - write_tsc_info
  - x86_pv_domain_info
  - x86_pv_map_m2p
  - xc_inflate_buffer
  - xc_read_image
  - xc_try_lz4_decode

The map files highlight a number of namespacing inconsistencies
(particularly with libxenguest using xc_* a significant amount).

It also seems to highlight a bunch of libxenguest.so functionalty
which appears to want to be exported (xc_*) but is not used in tree.
The initial list was based on what was needed to compile everything in
tree. I then looked through the list for xc_* and checked if any were
exported in a public header, leading to adding the following functions
which are intended to be public but not used in tree to the
libxenguest.map:
  - xc_cpuid_to_str
  - xc_compression_add_page
  - xc_compression_compress_pages
  - xc_compression_create_context
  - xc_compression_free_context
  - xc_compression_reset_pagebuf
  - xc_compression_uncompress_page

The remainder are internal functions using the external namespace (of
another library, no less).

Since the relevant libfoo.map is added to the Makefile dependencies of
the library, no longer use $^ to get the objects for libxenctrl and
libxenstore, libxenguest already didn't.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
This was something of an academic exercise, I'm not sure it's really
the right thing, especially for the unstable libraries.
---
 tools/libxc/Makefile           |  8 +++++---
 tools/libxc/libxenctrl.map     | 18 ++++++++++++++++++
 tools/libxc/libxenguest.map    | 37 +++++++++++++++++++++++++++++++++++++
 tools/xenstore/Makefile        |  5 +++--
 tools/xenstore/libxenstore.map | 15 +++++++++++++++
 5 files changed, 78 insertions(+), 5 deletions(-)
 create mode 100644 tools/libxc/libxenctrl.map
 create mode 100644 tools/libxc/libxenguest.map
 create mode 100644 tools/xenstore/libxenstore.map

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 940708f..8882aa1 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -206,8 +206,9 @@ libxenctrl.so: libxenctrl.so.$(MAJOR)
 libxenctrl.so.$(MAJOR): libxenctrl.so.$(MAJOR).$(MINOR)
 	$(SYMLINK_SHLIB) $< $@
 
-libxenctrl.so.$(MAJOR).$(MINOR): $(CTRL_PIC_OBJS)
-	$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(LDLIBS_libxentoollog) $(DLOPEN_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
+libxenctrl.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenctrl.map
+libxenctrl.so.$(MAJOR).$(MINOR): $(CTRL_PIC_OBJS) libxenctrl.map
+	$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenctrl.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(CTRL_PIC_OBJS) $(LDLIBS_libxentoollog) $(DLOPEN_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
 
 # libxenguest
 
@@ -228,8 +229,9 @@ endif
 xc_dom_bzimageloader.o: CFLAGS += $(call zlib-options,D)
 xc_dom_bzimageloader.opic: CFLAGS += $(call zlib-options,D)
 
+libxenguest.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenguest.map
 libxenguest.so.$(MAJOR).$(MINOR): COMPRESSION_LIBS = $(call zlib-options,l)
-libxenguest.so.$(MAJOR).$(MINOR): $(GUEST_PIC_OBJS) libxenctrl.so
+libxenguest.so.$(MAJOR).$(MINOR): $(GUEST_PIC_OBJS) libxenctrl.so libxenguest.map
 	$(CC) $(LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenguest.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $(GUEST_PIC_OBJS) $(COMPRESSION_LIBS) -lz $(LDLIBS_libxenctrl) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
 
 xenctrl_osdep_ENOSYS.so: $(OSDEP_PIC_OBJS) libxenctrl.so
diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map
new file mode 100644
index 0000000..cc93a5b
--- /dev/null
+++ b/tools/libxc/libxenctrl.map
@@ -0,0 +1,18 @@
+{
+	global:
+		xc_*;
+
+		/*
+		 * Supposedly internal functions which are also used
+		 * by libxenguest (only, it seems)
+		 */
+		read_exact;
+		write_exact;
+		writev_exact;
+
+		/* Other un-namespaced functions used elsewhere in tree */
+		do_xen_hypercall;
+		do_memory_op;
+
+	local: *; /* Do not expose anything by default */
+};
diff --git a/tools/libxc/libxenguest.map b/tools/libxc/libxenguest.map
new file mode 100644
index 0000000..5aa3605
--- /dev/null
+++ b/tools/libxc/libxenguest.map
@@ -0,0 +1,37 @@
+{
+	global:
+		xg_*;
+		elf_*; /* Stuff from libelf */
+		xc_dom_*; /* Domain builder */
+		xc_linux_build; /* Compat for xc_dom_* */
+
+		/* Either wrongly namespaced or in the wrong library */
+		xc_elf_set_logfile;
+
+		xc_domain_save;
+		xc_domain_restore;
+
+		xc_exchange_page;
+		xc_mark_page_online;
+		xc_mark_page_offline;
+		xc_query_page_offline_status;
+
+		xc_await_suspend;
+		xc_suspend_evtchn_init_exclusive;
+		xc_suspend_evtchn_init_sane;
+		xc_suspend_evtchn_release;
+
+		xc_map_m2p;
+
+		xc_dom_check_gzip;
+		xc_dom_do_gunzip;
+
+		xc_cpuid_apply_policy;
+		xc_cpuid_set;
+		xc_cpuid_check;
+		xc_cpuid_to_str;
+
+		xc_domain_get_native_protocol;
+
+	local: *; /* Do not expose anything by default */
+};
diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 1b4a494..87c2719 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -102,8 +102,9 @@ libxenstore.so.$(MAJOR): libxenstore.so.$(MAJOR).$(MINOR)
 
 xs.opic: CFLAGS += -DUSE_PTHREAD
 
-libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic
-	$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ $^ $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
+libxenstore.so.$(MAJOR).$(MINOR): SHLIB_LDFLAGS += -Wl,--version-script=libxenstore.map
+libxenstore.so.$(MAJOR).$(MINOR): xs.opic xs_lib.opic libxenstore.map
+	$(CC) $(LDFLAGS) $(PTHREAD_LDFLAGS) -Wl,$(SONAME_LDFLAG) -Wl,libxenstore.so.$(MAJOR) $(SHLIB_LDFLAGS) -o $@ xs.opic xs_lib.opic $(SOCKET_LIBS) $(PTHREAD_LIBS) $(APPEND_LDFLAGS)
 
 libxenstore.a: xs.o xs_lib.o
 	$(AR) rcs $@ $^
diff --git a/tools/xenstore/libxenstore.map b/tools/xenstore/libxenstore.map
new file mode 100644
index 0000000..c406207
--- /dev/null
+++ b/tools/xenstore/libxenstore.map
@@ -0,0 +1,15 @@
+{
+	global:
+		xs_*;
+
+		/*
+		 * Un-namespaced functions, may only be used from
+		 * xenstore_client.c? Unclear if they might be useful
+		 * from other potentially client applications.
+		 */
+		sanitise_value;
+		unsanitise_value;
+		expanding_buffer_ensure;
+
+		local: *; /* Do not expose anything by default */
+};
-- 
2.1.4

Re: [PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[Andrew Cooper]

On 07/01/16 13:51, Ian Campbell wrote:
> The map files highlight a number of namespacing inconsistencies
> (particularly with libxenguest using xc_* a significant amount).
>
> It also seems to highlight a bunch of libxenguest.so functionalty
> which appears to want to be exported (xc_*) but is not used in tree.
> The initial list was based on what was needed to compile everything in
> tree. I then looked through the list for xc_* and checked if any were
> exported in a public header, leading to adding the following functions
> which are intended to be public but not used in tree to the
> libxenguest.map:
>   - xc_cpuid_to_str
>   - xc_compression_add_page
>   - xc_compression_compress_pages
>   - xc_compression_create_context
>   - xc_compression_free_context
>   - xc_compression_reset_pagebuf
>   - xc_compression_uncompress_page

These compression functions became unused when I dropped legacy migration.

> diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map
> new file mode 100644
> index 0000000..cc93a5b
> --- /dev/null
> +++ b/tools/libxc/libxenctrl.map
> @@ -0,0 +1,18 @@
> +{
> +	global:
> +		xc_*;
> +
> +		/*
> +		 * Supposedly internal functions which are also used
> +		 * by libxenguest (only, it seems)
> +		 */
> +		read_exact;
> +		write_exact;
> +		writev_exact;

read/write_exact are used in libxc by xc_tmem.c, but only because the
tmem part of legacy migration split across the two libraries.

In the long term, they should move to being xenguest private.

~Andrew

> +
> +		/* Other un-namespaced functions used elsewhere in tree */
> +		do_xen_hypercall;
> +		do_memory_op;
> +
> +	local: *; /* Do not expose anything by default */
> +};

Re: [PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[Ian Campbell]

On Thu, 2016-01-07 at 14:36 +0000, Andrew Cooper wrote:
> On 07/01/16 13:51, Ian Campbell wrote:
> > The map files highlight a number of namespacing inconsistencies
> > (particularly with libxenguest using xc_* a significant amount).
> > 
> > It also seems to highlight a bunch of libxenguest.so functionalty
> > which appears to want to be exported (xc_*) but is not used in tree.
> > The initial list was based on what was needed to compile everything in
> > tree. I then looked through the list for xc_* and checked if any were
> > exported in a public header, leading to adding the following functions
> > which are intended to be public but not used in tree to the
> > libxenguest.map:
> >   - xc_cpuid_to_str
> >   - xc_compression_add_page
> >   - xc_compression_compress_pages
> >   - xc_compression_create_context
> >   - xc_compression_free_context
> >   - xc_compression_reset_pagebuf
> >   - xc_compression_uncompress_page
> 
> These compression functions became unused when I dropped legacy
> migration.

Indeed, I anticipated them either coming back or getting removed at some
point.

> > diff --git a/tools/libxc/libxenctrl.map b/tools/libxc/libxenctrl.map
> > new file mode 100644
> > index 0000000..cc93a5b
> > --- /dev/null
> > +++ b/tools/libxc/libxenctrl.map
> > @@ -0,0 +1,18 @@
> > +{
> > +	global:
> > +		xc_*;
> > +
> > +		/*
> > +		 * Supposedly internal functions which are also used
> > +		 * by libxenguest (only, it seems)
> > +		 */
> > +		read_exact;
> > +		write_exact;
> > +		writev_exact;
> 
> read/write_exact are used in libxc by xc_tmem.c, but only because the
> tmem part of legacy migration split across the two libraries.

They are also used by various osdep backend stuff in libxc, which should go
away with my split of those into other libraries and by xc_core.c (which is
in libxc, perhaps wrongly).

> In the long term, they should move to being xenguest private.
> 
> ~Andrew
> 
> > +
> > +		/* Other un-namespaced functions used elsewhere in
> > tree */
> > +		do_xen_hypercall;
> > +		do_memory_op;
> > +
> > +	local: *; /* Do not expose anything by default */
> > +};
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[Jan Beulich]

>>> On 07.01.16 at 14:51, <ian.campbell@citrix.com> wrote:
> --- /dev/null
> +++ b/tools/libxc/libxenctrl.map
> @@ -0,0 +1,18 @@
> +{
> +	global:

No actual version identifier / name space?

Jan

Re: [PATCH RFC] tools: add map files for libxen{store, ctrl, guest}.so

[Ian Campbell]

On Thu, 2016-01-07 at 07:49 -0700, Jan Beulich wrote:
> > > > On 07.01.16 at 14:51, <ian.campbell@citrix.com> wrote:
> > --- /dev/null
> > +++ b/tools/libxc/libxenctrl.map
> > @@ -0,0 +1,18 @@
> > +{
> > +	global:
> 
> No actual version identifier / name space?

Correct, this is only being used to hide internal symbols, not to to
provide any kind of ABI stability (which libxc and libxg lack).

Maybe libxenstore should be a) done separately and b) have a namespace as
you suggest and be declared stable in the same way as the things I am
adding under tools/libs are, but given that it uses libxenctrl there's
probably a little more work to do before that becomes possible.

Ian.