(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
cmt...@google.com
On Tue, Nov 24, 2015 at 2:24 AM, Rainer Orth
<r...@cebitec.uni-bielefeld.de> wrote:
> Rainer Orth <r...@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 <r...@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
>
