(no subject)

[david]

Here is my proposal, based on Clint's suggestion on IRC, and on the
sed hack from librhash, to hide all symbols that are not explicitly
part of the notmuch API. In particular it hides the various symbols
related to Xapian exceptions.

[PATCH 1/2] libnotmuch: add linker script to declare only notmuch_* symbols as global.

[david]

From: David Bremner <bremner@debian.org>

This is closely tied to gcc and particularly gnu ld, but I guess the
shared library linking code would need to be adjusted to work on a
non-gnu linker anyay.

I had to make a few not-obviously related changes to the
lib/Makefile.local to make this work: libnotmuch_modules is defined
with := and used in place of $^
---
 lib/Makefile.local |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 4319023..4676504 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -36,7 +36,7 @@ LIBRARY_SUFFIX = so
 LINKER_NAME = libnotmuch.$(LIBRARY_SUFFIX)
 SONAME = $(LINKER_NAME).$(LIBNOTMUCH_VERSION_MAJOR)
 LIBNAME = $(SONAME).$(LIBNOTMUCH_VERSION_MINOR).$(LIBNOTMUCH_VERSION_RELEASE)
-LIBRARY_LINK_FLAG = -shared -Wl,-soname=$(SONAME)
+LIBRARY_LINK_FLAG = -shared -Wl,--version-script=notmuch.sym,-soname=$(SONAME)
 ifeq ($(LIBDIR_IN_LDCONFIG),1)
 ifeq ($(DESTDIR),)
 LIBRARY_INSTALL_POST_COMMAND=ldconfig
@@ -66,13 +66,20 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/query.cc		\
 	$(dir)/thread.cc
 
-libnotmuch_modules = $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
+libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
 
 $(dir)/libnotmuch.a: $(libnotmuch_modules)
 	$(call quiet,AR) rcs $@ $^
 
-$(dir)/$(LIBNAME): $(libnotmuch_modules)
-	$(call quiet,CXX $(CXXFLAGS)) $^ $(FINAL_LIBNOTMUCH_LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@
+$(dir)/$(LIBNAME): $(libnotmuch_modules) notmuch.sym
+	echo $(libnotmuch_modules)
+	$(call quiet,CXX $(CXXFLAGS)) $(libnotmuch_modules) $(FINAL_LIBNOTMUCH_LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@
+
+notmuch.sym: lib/notmuch.h
+	gcc -aux-info notmuch.aux $<
+	printf "{\nglobal:\n" > notmuch.sym
+	sed  -n 's/.*\(notmuch_[a-z_]*\) (.*/\t\1;/p' notmuch.aux >> notmuch.sym
+	printf "local: *;\n};\n" >> notmuch.sym
 
 $(dir)/$(SONAME): $(dir)/$(LIBNAME)
 	ln -sf $(LIBNAME) $@
@@ -96,4 +103,4 @@ install-$(dir): $(dir)/$(LIBNAME)
 	$(LIBRARY_INSTALL_POST_COMMAND)
 
 SRCS  := $(SRCS) $(libnotmuch_c_srcs) $(libnotmuch_cxx_srcs)
-CLEAN := $(CLEAN) $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME) $(dir)$(LIBNAME) libnotmuch.a
+CLEAN := $(CLEAN) $(libnotmuch_modules) $(dir)/$(SONAME) $(dir)/$(LINKER_NAME) $(dir)$(LIBNAME) libnotmuch.a notmuch.aux notmuch.sym
-- 
1.7.5.3

[PATCH 2/2] remove Xapian exceptions symbols from libnotmuch1.symbols

[david]

From: David Bremner <bremner@debian.org>

These were never intended to be public, since the library exports only
a C API.
---
 debian/libnotmuch1.symbols |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/debian/libnotmuch1.symbols b/debian/libnotmuch1.symbols
index f50d168..8df6fec 100644
--- a/debian/libnotmuch1.symbols
+++ b/debian/libnotmuch1.symbols
@@ -1,18 +1,4 @@
 libnotmuch.so.1 libnotmuch1 #MINVER#
- _Z32_notmuch_message_ensure_metadataP16_notmuch_message@Base 0.6~171
- _ZN6Xapian20InvalidArgumentErrorC1ERKS0_@Base 0.6~171
- _ZN6Xapian20InvalidArgumentErrorC2ERKS0_@Base 0.6~171
-#MISSING: 0.6~180# _ZN6Xapian20InvalidArgumentErrorD1Ev@Base 0.6~171
- _ZTIN6Xapian10LogicErrorE@Base 0.3
- _ZTIN6Xapian12RuntimeErrorE@Base 0.3
- _ZTIN6Xapian16DocNotFoundErrorE@Base 0.3
- _ZTIN6Xapian20InvalidArgumentErrorE@Base 0.3
- _ZTIN6Xapian5ErrorE@Base 0.3
- _ZTSN6Xapian10LogicErrorE@Base 0.3
- _ZTSN6Xapian12RuntimeErrorE@Base 0.3
- _ZTSN6Xapian16DocNotFoundErrorE@Base 0.3
- _ZTSN6Xapian20InvalidArgumentErrorE@Base 0.3
- _ZTSN6Xapian5ErrorE@Base 0.3
  notmuch_database_add_message@Base 0.3
  notmuch_database_close@Base 0.3
  notmuch_database_create@Base 0.3
-- 
1.7.5.3

Re: hiding xapian symbols

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

On 06/21/2011 09:08 PM, david@tethera.net wrote:
> Here is my proposal, based on Clint's suggestion on IRC, and on the
> sed hack from librhash, to hide all symbols that are not explicitly
> part of the notmuch API. In particular it hides the various symbols
> related to Xapian exceptions.

I like this proposal -- i would be really glad to not see the mangled
Xapian symbols in libnotmuch.so.

However, i'm concerned that the act of hiding these symbols will make it
so that any program that tries to link both libnotmuch and libxapian
will be unable to use these symbols.

Consider the visiblity rules [0]:

>> Symbol visibility is "default" by default but if the linker encounters 
>> just one definition with it hidden - just one - that typeinfo symbol 
>> becomes permanently hidden (remember the C++ standard's ODR - one 
>> definition rule). 


This makes me think that a program that links against libnotmuch and
libxapian, but actually tries to *use* libxapian's exception handling
may have a problem.

This is probably best resolved by writing such an executable and seeing
if it behaves, but alas, i haven't done that work.   Any takers?
Perhaps this could be a test in the notmuch test suite?

	--dkg


[0]
http://gcc.gnu.org/wiki/Visibility#Problems_with_C.2B-.2B-_exceptions_.28please_read.21.29



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 1030 bytes --]

Re: hiding xapian symbols

[David Bremner]

On Tue, 21 Jun 2011 22:05:15 -0400, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> However, i'm concerned that the act of hiding these symbols will make it
> so that any program that tries to link both libnotmuch and libxapian
> will be unable to use these symbols.
> 
> This makes me think that a program that links against libnotmuch and
> libxapian, but actually tries to *use* libxapian's exception handling
> may have a problem.
> 

This seems to work, when compiled "g++ foo.c -lnotmuch -lxapian"

I get 

,----
| Error opening database at /nonexistant/.notmuch: No such file or directory
| caught Cannot create directory `/nonexistant'
`----

don't run it as root ;)

#include <stdio.h>
#include <xapian.h>
#include <notmuch.h>
main (int argc, char **argv){

    notmuch_database_t *notmuch
      = notmuch_database_open ("/nonexistant",
				     NOTMUCH_DATABASE_MODE_READ_ONLY);

  try{
    (void)new Xapian::WritableDatabase ("/nonexistant",
					Xapian::DB_CREATE_OR_OPEN);
  } catch (const Xapian::Error &error) {
    printf("caught %s\n",error.get_msg().c_str());
  }
}

[PATCH] tests: add a test for symbol hiding side effects

[david]

From: David Bremner <bremner@debian.org>

The worry here is that a binary linking with libnotmuch might lose
access to Xapian::Error symbols because libnotmuch hides them.
---
 test/basic          |    2 +-
 test/notmuch-test   |    1 +
 test/symbol-hiding  |   21 +++++++++++++++++++++
 test/symbol-test.cc |   17 +++++++++++++++++
 4 files changed, 40 insertions(+), 1 deletions(-)
 create mode 100755 test/symbol-hiding
 create mode 100644 test/symbol-test.cc

diff --git a/test/basic b/test/basic
index 808c968..d6e8c10 100755
--- a/test/basic
+++ b/test/basic
@@ -56,7 +56,7 @@ tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
 available=$(ls -1 ../ | \
     sed -r -e "/^(aggregate-results.sh|Makefile|Makefile.local|notmuch-test)/d" \
 	   -e "/^(README|test-lib.sh|test-lib.el|test-results|tmp.*|valgrind|corpus*)/d" \
-	   -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose)/d" \
+	   -e "/^(emacs.expected-output|smtp-dummy|smtp-dummy.c|test-verbose|symbol-test.cc)/d" \
 	   -e "/^(test.expected-output|.*~)/d" \
 	   -e "/^(gnupg-secret-key.asc)/d" \
 	   -e "/^(gnupg-secret-key.NOTE)/d" \
diff --git a/test/notmuch-test b/test/notmuch-test
index 83f284d..fe85c6a 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -40,6 +40,7 @@ TESTS="
   emacs-large-search-buffer
   maildir-sync
   crypto
+  symbol-hiding
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
diff --git a/test/symbol-hiding b/test/symbol-hiding
new file mode 100755
index 0000000..1c1f34e
--- /dev/null
+++ b/test/symbol-hiding
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2011 David Bremner
+#
+
+test_description='exception symbol test
+
+This test tests whether hiding Xapian::Error symbols in libnotmuch
+also hides them for other users of libxapian. This is motivated by
+the discussion in http://gcc.gnu.org/wiki/Visibility'
+
+. ./test-lib.sh
+
+output="Error opening database at /nonexistant/.notmuch: No such file or directory
+caught No chert database found at path \`/nonexistant'"
+
+g++ -o symbol-test ../symbol-test.cc -L../lib -lnotmuch -lxapian
+test_expect_success "$message" ./symbol-test
+test_done
+
+
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
new file mode 100644
index 0000000..f077845
--- /dev/null
+++ b/test/symbol-test.cc
@@ -0,0 +1,17 @@
+#include <stdio.h>
+#include <xapian.h>
+#include <notmuch.h>
+main (int argc, char **argv){
+
+    notmuch_database_t *notmuch
+      = notmuch_database_open ("/nonexistant",
+				     NOTMUCH_DATABASE_MODE_READ_ONLY);
+
+  try{
+    (void)new Xapian::WritableDatabase ("/nonexistant",
+					Xapian::DB_OPEN);
+  } catch (const Xapian::Error &error) {
+    printf("caught %s\n",error.get_msg().c_str());
+  }
+  return 0;
+}
-- 
1.7.5.3

Re: [PATCH] tests: add a test for symbol hiding side effects

[David Bremner]

On Wed, 22 Jun 2011 09:02:36 -0300, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
> 
> The worry here is that a binary linking with libnotmuch might lose
> access to Xapian::Error symbols because libnotmuch hides them.

After some helpful feedback from jrollins and amdragon on IRC, I have
pushed an updated version of this. The guts of the updated test are as
follows:

run_test(){
    result=$(LD_LIBRARY_PATH=../../lib ./symbol-test 2>&1)
}

output="A Xapian exception occurred opening database: Couldn't stat 'fakedb/.notmuch/xapian'
caught No chert database found at path \`./nonexistant'"

g++ -o symbol-test -I../../lib ../symbol-test.cc -L../../lib -lnotmuch -lxapian
mkdir -p fakedb/.notmuch
test_expect_success 'running test' run_test
test_begin_subtest 'checking output'
test_expect_equal "$result" "$output" 
test_done

Re: [PATCH] tests: add a test for symbol hiding side effects

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

On Thu, 23 Jun 2011 07:24:08 -0300, David Bremner <david@tethera.net> wrote:
> On Wed, 22 Jun 2011 09:02:36 -0300, david@tethera.net wrote:
> > From: David Bremner <bremner@debian.org>
> > 
> > The worry here is that a binary linking with libnotmuch might lose
> > access to Xapian::Error symbols because libnotmuch hides them.
> 
> After some helpful feedback from jrollins and amdragon on IRC, I have
> pushed an updated version of this. The guts of the updated test are as
> follows:

Lovely stuff! I so love having new features come along with their own tests.

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]