Solaris & [PATCH libdrm 1/2] configure.ac: split -fvisibility and __attribute__((visibility)) checks

[Emil Velikov]

On 23/03/15 01:46, Alan Coopersmith wrote:
> On 03/ 9/15 05:37 AM, Emil Velikov wrote:
>> The former does not imply the latter and vice-versa. One such example is
>> the Sun compiler.
>>
>> Cc: Alan Coopersmith <alan.coopersmith at oracle.com>
>> Cc: Thierry Reding <treding at nvidia.com>
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>
>> Hi Alan,
>> Can you please take a look it this series covers the symbol visibility
>> issues mentioned earlier ? In theory it should work like a charm but I
>> cannot really test it :-\
> 
> Patch 1 of 2 breaks configure on Solaris - I get:
> 
> checking for VALGRIND... no
> checking whether cc -xc99=%all supports __attribute__((visibility))...
> ./configure[13393]: set does not quote correctly, so add quotes:
> double-quote^J      # substitution turns \\ into \, and sed turns \ into
> \.^J      sed -n ^I"s//\\/g;^J^I 
> s/^\([_$as_cr_alnum]*_cv_[_$as_cr_alnum]*\)=\(.*\)/\1=2/p"^J      ;;
> #(^J    *)^J      # : not found [No such file or directory]
> ./configure[13393]: {\1+set}: bad substitution
> ./configure[13992]: : cannot open
> ./configure[14000]: : cannot open
> ./configure[14032]: : cannot open
> ./configure[14051]: : cannot open
> ./configure[14128]: : cannot open
> ./configure[14139]: : cannot open
> ./configure[14150]: : cannot open
> ./configure[14435]: : cannot open
> ./configure[14563]: : cannot open
> ./configure[14603]: : cannot open
> ./configure[14610]: : cannot open
> ./configure[14639]: : cannot open
> ./configure[14671]: : cannot open
> ./configure[14740]: : cannot open
> ./configure[14744]: : cannot open
> ./configure[14778]: : cannot open
> ./configure[14928]: : cannot open
> ./configure[14948]: : cannot open
> ./configure[14962]: : cannot open
> ./configure[14966]: : cannot open
> configure: error: write failure creating
> It also generated new autoconf warnings generating the configure script:
> 
> configure.ac:422: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:193: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2661: _AC_LINK_IFELSE is expanded from...
> ../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
> configure.ac:422: the top level
> configure.ac:422: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:193: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2661: _AC_LINK_IFELSE is expanded from...
> ../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
> configure.ac:422: the top level
> autoreconf: running:
> /net/also.us.oracle.com/export/alanc/autotools/install/bin/autoconf --force
> configure.ac:422: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:193: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2661: _AC_LINK_IFELSE is expanded from...
> ../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
> configure.ac:422: the top level
> autoreconf: running:
> /net/also.us.oracle.com/export/alanc/autotools/install/bin/autoheader
> --force
> configure.ac:422: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:193: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2661: _AC_LINK_IFELSE is expanded from...
> ../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
> configure.ac:422: the top level
> autoreconf: running: automake --add-missing --copy --force-missing
> configure.ac:422: warning: AC_LANG_CONFTEST: no AC_LANG_SOURCE call
> detected in body
> ../../lib/autoconf/lang.m4:193: AC_LANG_CONFTEST is expanded from...
> ../../lib/autoconf/general.m4:2661: _AC_LINK_IFELSE is expanded from...
> ../../lib/autoconf/general.m4:2678: AC_LINK_IFELSE is expanded from...
> configure.ac:422: the top level
> 
> Both seem to have been caused by a misplaced close paren/brace pair and
> are fixed by:
> 
> --- a/configure.ac
> +++ b/configure.ac
> @@ -422,7 +422,7 @@ AC_MSG_CHECKING([whether $CC supports
> __attribute__((visibility))])
> AC_LINK_IFELSE([AC_LANG_PROGRAM([
>     int foo_default( void ) __attribute__((visibility("default")));
>     int foo_hidden( void ) __attribute__((visibility("hidden")));
> -], HAVE_ATTRIBUTE_VISIBILITY="yes"; AC_MSG_RESULT([yes]),
> AC_MSG_RESULT([no])]);
> +])], [HAVE_ATTRIBUTE_VISIBILITY="yes"; AC_MSG_RESULT([yes])],
> AC_MSG_RESULT([no]));
> 
> if test "x$HAVE_ATTRIBUTE_VISIBILITY" = xyes; then
>     AC_DEFINE(HAVE_VISIBILITY, 1, [Compiler supports
> __attribute__((visibility))])
> 
> (Gotta love autoconf and the mysteries of proper [] placements.)
> 
Playing around with [] is always fun. Thanks for the fix.

> After that I can build on Solaris with patch #1 applied, but patch #2
> then breaks
> due to Solaris Studio 12.4 compilers being more pedantic about
> prototypes in headers
> having the external visibility declarations too - lots of errors of the
> form:
> 
> "intel_bufmgr.c", line 60: redeclaration must have the same or more
> restrictive linker scoping: drm_intel_bo_alloc_for_render
> 
> It looks like none of the headers have the drm_public attributes the
> compiler wants
> to see there.
> 
Ouch... this explains why many places in X symbols are explicitly
annotated as X_HIDDEN. From a closer look it seems to be the
shorter/safe option. Plus to catch internal symbols slipping away I can
add wire up a few tests at make check :-)

>> Additionally can you guys build libdrm (barring the patch you sent the
>> other day), or you need some parts of this ancient patch
>> http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/x11-libs/libdrm/files/libdrm-2.4.58-solaris.patch?view=markup
>>
> 
> That looks very much like an ancient patch we still have in our tree [1]
> since
> I never found out if we could upstream it or replace it from our former DRM
> team. Unfortunately, they're all gone now - I've cc'ed the people who
> picked
> up that work - I'd already asked Randy to look at it as part of the work
> he's
> doing to reconcile our DRM sources with upstream.
> 
> [1]
> https://hg.java.net/hg/solaris-x11~x-s12-clone/file/4e6b5865e3ec/open-src/lib/libdrm/solaris-drm-port.patch
> 
> 
> I keep a subset in my local git repo to enable building from upstream
> since the
> raw upstream builds don't work without at least some of it.   (See
> attached patches for that subset.)
> 
Thanks !

Was honestly hoping that patch #1 is not required as:
 - Getting the drm.h header in sync with the kernel will be annoying.
 - The struct drm_map/drmMapBufs/drmRmMap is part of the legacy drm
cruft for which, I would like to think, there are no more users.

Obviously the latter can be confirmed by Randy and friends.

Cheers,
Emil