Re: Port libvtv to Solaris

[Caroline Tice <cmtice at google dot com>]

(Trying this again; the mailer daemon rejected my first message)

All of the patch looks good to me, but I can only approve the libvtv
portion (which I do).  Someone else will need to approve the rest.

-- Caroline Tice
cmtice@google.com


On Tue, Nov 24, 2015 at 2:24 AM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:
>
>> Now that init priority support on Solaris is on mainline, porting libvtv
>> proved to be relatively easy, though it discovered a couple of quirks on
>> a non-gld non-x86 platform.
>
> This patch has now remained unreviewed for a week.  With both Jeff and
> Richi fine with it going into mainline even this late, it would be nice
> if Caroline could find the time to review it.
>
> Linux/x86_64 testing found one minor issue, explained below.  I'm
> attaching the updated patch.
>
>> A considerable part of the patch lives in Solaris-specific files and
>> thus doesn't need approval, though some changes require explanation:
>>
>> * In gcc.c (LINK_COMMAND_SPEC), VTABLE_VERIFICATION_SPEC was before
>>   %{L*}.  The spec includes -lvtv.  Solaris ld, other than GNU ld, heeds
>>   the relative order of -L and -l switches, so the libvtv testcases
>>   wouldn't link manually, but did inside a testsuite run where
>>   LD_LIBRARY_PATH points ld at the correct directory.
>>
>> * Solaris/SPARC uses an 8 kB page size, so a couple of cases where uses
>>   of VTV_PAGE_SIZE had been replaced with hardcoded values of 4096 had
>>   to be reverted.
>>
>> * Inside libgcc, the vtv_*.c files are compiled with
>>   -finhibit-size-directive, whereas in libvtv that flag is absent.  This
>>   caused all testcases to fail due to a linker warning:
>>
>> FAIL: libvtv.cc/bb_tests.cc -O0 -fvtable-verify=std (test for excess errors)
>> Excess errors:
>> ld: warning: symbol '_vtable_map_vars_end' has differing sizes:
>>         (file /var/gcc/gcc-6.0.0-20151111/12-gcc-vtv/sparc-sun-solaris2.12/./libvtv/.libs/libvtv.so value=0x1000; file /var/gcc/gcc-6.0.0-20151111/12-gcc-vtv/gcc/vtv_end.o value=0x0);
>>         /var/gcc/gcc-6.0.0-20151111/12-gcc-vtv/gcc/vtv_end.o definition taken
>>
>> * Like Cygwin, Solaris has no obstack functions in libc, so I'm now
>>   using a common conditional for that.
>>
>> * libvtv requires constructor priority support and dl_iterate_phdr.  The
>>   former needs either a recent (Solaris 12 only so far) ld or gld, the
>>   latter came in Solaris 11 only.
>>
>> * Unlike glibc systems, Solaris has no __fortify_fail in libc; some of
>>   this can probably provided using libbacktrace, which I haven't yet
>>   done.
>>
>> * It also lacks program_invocation_name, but the functionality can be
>>   provided via getexecname() instead.
>>
>> * On Solaris 12/SPARC with Solaris as, the .vtable_map_vars section
>>   wouldn't be pagesize aligned (I'm still looking how to fix this; it
>>   works out of the box for .bss), so the section length calclated in
>>   read_section_offset_and_length would be negative, leading to all sorts
>>   of havoc.  For the moment, I'm using gas instead to avoid this.
>>
>> * The patch also fixes a number of typos noticed during testing.
>>
>> With those changes, I get almost clean libvtv test results on
>> i386-pc-solaris2.12 (as/ld and gas/gld) and sparc-sun-solaris2.12
>> (gas/ld):
>>
>> * i386-pc-solaris2.12:
>>
>>               === libvtv tests ===
>>
>>
>> Running target unix
>>
>>               === libvtv Summary for unix ===
>>
>> # of expected passes          176
>>
>> Running target unix/-m64
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O0 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O2 -fvtable-verify=std -lpthread execution test
>>
>>               === libvtv Summary for unix/-m64 ===
>>
>> # of expected passes          174
>> # of unexpected failures      2
>>
>>               === libvtv Summary ===
>>
>> # of expected passes          350
>> # of unexpected failures      2
>>
>> * sparc-sun-solaris2.12:
>>
>>               === libvtv tests ===
>>
>>
>> Running target unix
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O0 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_mt.cc -O0 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O2 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_mt.cc -O2 -fvtable-verify=std -lpthread execution test
>>
>>               === libvtv Summary for unix ===
>>
>> # of expected passes          172
>> # of unexpected failures      4
>>
>> Running target unix/-m64
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O0 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_mt.cc -O0 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_inserts_mt.cc -O2 -fvtable-verify=std -lpthread execution test
>> WARNING: program timed out.
>> FAIL: libvtv.mt.cc/register_set_pair_mt.cc -O2 -fvtable-verify=std -lpthread execution test
>>
>>               === libvtv Summary for unix/-m64 ===
>>
>> # of expected passes          172
>> # of unexpected failures      4
>>
>>               === libvtv Summary ===
>>
>> # of expected passes          344
>> # of unexpected failures      8
>>
>> I'm still investigating what causes those timeouts, it seems to be a
>> scalability issue in libc.
>>
>> While I realize that we are past stage1, maybe the fact that this patch
>> is for an off-by-default feature and well localized still could allow it
>> into mainline at this point?
>
> On x86_64-pc-linux-gnu, initial testing found all libvtv testcases
> failing initially like this:
>
> FAIL: libvtv.cc/bb_tests.cc -O0 -fvtable-verify=std (test for excess errors)
> Excess errors:
> vtv_end.c:(.text.startup+0x0): multiple definition of `__VLTprotect'
> /var/gcc/gcc-6.0.0-20151111/3.19.8-gcc-gas-gld-vtv/x86_64-pc-linux-gnu/./libvtv/.libs/libvtv.a(vtv_end.o):/var/gcc/gcc-6.0.0-20151111/3.19.8-gcc-gas-gld-vtv/x86_64-pc-linux-gnu/libvtv/vtv_end.c:59: first defined here
>
> It turned out that while the libvtv.so.0 and libvtv.so symlinks were
> built, libvtv.so.0.0.0 proper was not.  I'd overlooked this aucotonf
> warning:
>
> ro@lokon 1503 > autoconf
> configure.ac:109: warning: AC_COMPILE_IFELSE was called before AC_USE_SYSTEM_EXTENSIONS
> /vol/src/gnu/autoconf/autoconf-2.64/lib/autoconf/specific.m4:332: AC_GNU_SOURCE is expanded from...
> configure.ac:109: the top level
>
> As expected, adding the macro to configure.ac fixed both the warning and
> the libvtv.so.0.0.0 build, so Linux/x86_64 results are all fine again.
>
> Thanks.
>
>         Rainer
>
> 2015-08-20  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>
>         libstdc++-v3:
>         * acinclude.m4 (GLIBCXX_ENABLE_VTABLE_VERIFY) <solaris2*>: Use
>         -Wl,-R in VTV_CXXLINKFLAGS.
>         * configure: Regenerate.
>
>         * testsuite/18_support/bad_exception/23591_thread-1.c: Use
>         -fvtable-verify=none on Solaris 12+.
>
>         libgcc:
>         * Makefile.in (VTV_CFLAGS): New variable.
>         (vtv_start$(objext), vtv_end$(objext), vtv_end$(objext))
>         (vtv_start_preinit$(objext), vtv_end_preinit$(objext)): Use it.
>         * config.host (*-*-solaris2*): Add t-crtstuff-pic to tmake_file.
>         Add vtv_start.o, vtv_end.o, vtv_start_preinit.o, vtv_end_preinit.o
>         to extra_parts if $enable_vtable_verify = yes.
>
>         libvtv:
>         * configure.tgt (*-*-solaris2.[1-9]*): Declare supported.
>         * configure.ac: Call AC_USE_SYSTEM_EXTENSIONS.
>         <*-*-solaris2*>: Check for init priority support.
>         Check for getexecname, __fortify_fail, _obstack_begin.
>         (VTV_NO_OBSTACK): New conditional.
>         * configure: Regenerate.
>         * Makefile.am [VTV_NO_OBSTACK] (obstack.c): Use new condition.
>         * Makefile.in: Regenerate.
>
>         * vtv_rts.cc [HAVE_GETEXECNAME] (program_invocation_name): New
>         variable.
>         (read_section_offset_and_length) [HAVE_GETEXECNAME]: Set it.
>         (dl_iterate_phdr_callback) [HAVE_GETEXECNAME]: Set it.
>
>         (__fortify_fail): Wrap in HAVE___FORTIFY_FAIL
>         [!HAVE___FORTIFY_FAIL]: Provide non-Cygwin implementation.
>
>         (read_section_offset_and_length): Assert sh_size >= VTV_PAGE_SIZE.
>         (iterate_modules): Fix typo.
>         Use VTV_PAGE_SIZE.
>         (dl_iterate_phdr_callback): Fix typo.
>         Use VTV_PAGE_SIZE.
>         (__VLTChangePermission): Fix typos.
>
>         include:
>         * vtv-change-permission.h (VTV_PAGE_SIZE) [__sun__ && __svr4__ &&
>         __sparc__]: Define.
>
>         gcc:
>         * config/sol2.h (SUPPORTS_INIT_PRIORITY): Move up.
>         (STARTFILE_VTV_SPEC, ENDFILE_VTV_SPEC): Define.
>         (STARTFILE_SPEC): Use %(startfile_vtv).
>         (ENDFILE_SPEC): Use %(endfile_vtv).
>         (SUBTARGET_EXTRA_SPECS): Handle STARTFILE_VTV_SPEC,
>         ENDFILE_VTV_SPEC.
>
>         * gcc.c (LINK_COMMAND_SPEC): Move VTABLE_VERIFICATION_SPEC after %{L*}.
>
>
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
>