On 27 April 2016 at 08:48, Yury Gribov <[email protected]> wrote: > On 04/26/2016 02:14 PM, Emil Velikov wrote: >> >> On 26 April 2016 at 07:54, Yury Gribov <[email protected]> wrote: >>> >>> On 04/20/2016 10:03 AM, Yury Gribov wrote: >>>> >>>> >>>> On 04/19/2016 06:35 PM, Yury Gribov wrote: >>>>> >>>>> >>>>> On 04/18/2016 06:21 PM, Adam Jackson wrote: >>>>>> >>>>>> >>>>>> On Mon, 2016-04-18 at 09:23 +0300, Yury Gribov wrote: >>>>>> >>>>>>> Does below look like a sane approach? >>>>>>> 1) get all deps via >>>>>>> $ apt-cache rdepends libice6 libice-dev >>>>>>> 2) unpack each of the above .debs and scan for ELFs >>>>>>> 3) for each ELF see if one of now hidden symbols is UND >>>>>> >>>>>> >>>>>> >>>>>> That sounds good to me. >>>>> >>>>> >>>>> >>>>> I've cooked a simple script (attached, reviews welcome) and applied it >>>>> to Ubuntu 14. It showed that there are additional uses for >>>>> _IceTransNoListen (mentioned by Alan in separate email) but nothing >>>>> else. >>>>> >>>>>>> Note that this does not handle transitive dependencies e.g. if some >>>>>>> weird library links static version of libICE and the re-exports it's >>>>>>> symbols as part of new lib's public interface. >>>>>> >>>>>> >>>>>> >>>>>> It'd be possible to scan for this too I suspect. Look for packages >>>>>> that BuildRequire libice6-static, scan the resulting binaries for >>>>>> exports. I suspect there are zero such packages. >>>>> >>>>> >>>>> >>>>> I have postponed this activity for now (especially given that this >>>>> behavior would be a serious packaging abuse). >>>> >>>> >>>> >>>> Here's an updated patch with added _IceTransNoListen. Does this look >>>> better now? >>> >>> >>> >>> Ping. >>> >> Pong. Sending patches as attachments is not the most productive way to >> get things reviewed ;-) > > > I see. Strangely the Content-Disposition in my previous email is properly > set to "inline". I've both inlined and attached the new patch to be sure. > Close but no cigar :-P Please follow the suggestions in the wiki http://www.x.org/wiki/Development/Documentation/SubmittingPatches/
>> But seriously, you want to update the configure.ac to handle more than >> just GCC. Feel free to copy some/most of the logic from xserver's >> configure.ac > > > Right, fixed. I initially decided to not copy the visibility detection hunk > from other places because was unsure of licenses and stuff. But xserver > license is obviously compatible so should be fine. > >> Emil >> >> > > > From 40fa9c6eb6fbaa924bf90efcefef681bbaab4194 Mon Sep 17 00:00:00 2001 > From: Yury Gribov <[email protected]> > Date: Tue, 12 Apr 2016 17:04:09 +0300 > Subject: [PATCH] Added visibility annotations. > > Signed-off-by: Yury Gribov <[email protected]> > --- > configure.ac | 42 ++++++++++++++++++++++++ > include/X11/ICE/ICElib.h | 82 > +++++++++++++++++++++++------------------------ > include/X11/ICE/ICEmsg.h | 18 +++++------ > include/X11/ICE/ICEutil.h | 18 +++++------ > src/icetrans.c | 4 +++ > 5 files changed, 105 insertions(+), 59 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 458882a..f63da07 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -29,8 +29,13 @@ XORG_WITH_FOP > XORG_WITH_XSLTPROC > XORG_CHECK_SGML_DOCTOOLS(1.8) > > +AC_ARG_ENABLE(visibility, AS_HELP_STRING([--enable-visibility], [Enable > symbol visibility (default: auto)]), > + [SYMBOL_VISIBILITY=$enableval], > + [SYMBOL_VISIBILITY=auto]) > + > # Obtain compiler/linker options for depedencies > PKG_CHECK_MODULES(ICE, xproto xtrans) > +PKG_PROG_PKG_CONFIG > > # Transport selection macro from xtrans.m4 > XTRANS_CONNECTION_FLAGS > @@ -40,6 +45,43 @@ AC_DEFINE(ICE_t, 1, [Xtrans transport type]) > AC_CHECK_LIB([bsd], [arc4random_buf]) > AC_CHECK_FUNCS([asprintf arc4random_buf]) > > +have_visibility=disabled > +if test x$SYMBOL_VISIBILITY != xno; then > + AC_MSG_CHECKING(for symbol visibility support) > + if test x$GCC = xyes; then > + VISIBILITY_CFLAGS="-fvisibility=hidden" > + else > + if test x$SUNCC = xyes; then > + VISIBILITY_CFLAGS="-xldscope=hidden" > + else > + have_visibility=no If Cygwin/Mingw does support the visibility flags (see below) I'd just warn/error out in here. IMHO if a specific platform/compiler is not handles we better get a report and sort it out. In general I would not bother with anything more than - PKG_CHECK_MODULE(XPROTO, xproto) + wire up the XPROTO_CFLAGS (both missing) - Add the above "if test $compiler set VISIBILILTY_FLAGS" + propagate the latter throughout (both done) - Add (now missing) #include <X11/Xfuncproto.h> in the respective places and annotate the functions (already done) No extra configure toggles (above), no configure time testing. Unless cygwin/mingw requires the latter. > + fi > + fi > + if test x$have_visibility != xno; then > + save_CFLAGS="$CFLAGS" > + proto_inc=`$PKG_CONFIG --cflags xproto` > + CFLAGS="$CFLAGS $VISIBILITY_CFLAGS $proto_inc" > + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([ > + [#include <X11/Xfuncproto.h> > + #if defined(__CYGWIN__) || > defined(__MINGW32__) > + #error No visibility support Are you sure that think this is correct ? Afaict things should work, despite that xserver sets SYMBOL_VISIBILITY=no. Jon, any ideas why xserver disables -fvisibility=hidden ? Should we remove that line or there's something subtle that requires the behaviour ? For example: relying on symbols that are not marked as exported or alike. From past (ancient) experiences and reading the wiki correctly, combining -fivisiblity=hidden + dllimport/dllexport should work fine ? Thanks Emil [1] https://gcc.gnu.org/wiki/Visibility _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
