From patchwork Tue Jul 28 17:11:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Caroline Tice X-Patchwork-Id: 501314 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 40EE9140D25 for ; Wed, 29 Jul 2015 03:11:49 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=LGMO4+KH; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; q=dns; s=default; b=feI5fAIYLbil08m3wWL18euVjLVuf4b1g8u9Y8Wlufo vwqqJYo/xIdERKnkcNWrzUu+Yf8RERs38HaKuwKROU8yluez+9LB4x2ioXFCbIEz K9+AAPpVl7l3HxGPRF/x/bi/uJETCpEZVIdwom4o/4W5TceBdMOzs95MmjuwsxN8 = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:cc:content-type; s=default; bh=U6zdDxbLrOn0ns/B5Y+M71Pc090=; b=LGMO4+KHbaIFhQVKU bVOuHLeL2U0JkY+BneFL890C/HSEm/r8tzntc5Mm9CbvGNw6TnHJ4agSZl58VRQs y38B40Yfk8+2Fkv86MHOh9yRvWsyVOzYe9qXHC8SS77IweptLoSd+uFn8RyHnWge cxgcCM7kNEWYqu8WF/hFQ5G6No= Received: (qmail 3403 invoked by alias); 28 Jul 2015 17:11:43 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 3387 invoked by uid 89); 28 Jul 2015 17:11:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_05, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vn0-f47.google.com Received: from mail-vn0-f47.google.com (HELO mail-vn0-f47.google.com) (209.85.216.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 28 Jul 2015 17:11:40 +0000 Received: by vnav141 with SMTP id v141so45413126vna.0 for ; Tue, 28 Jul 2015 10:11:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc :content-type; bh=1NQUF32YJkaLHZyd4hahWbLWmiF16qlNxhptxdludek=; b=P7vuJZUO/iTw5RyFw0cjWXuCM0fIe+B5BFoOiqFFqSeQtey/O+ScDBbJUnAgnLHfSX K4rK0c7BLLZIsEfwEELqKJ/QWtctNjrQvqxS6HmXL8OGMGSOr79wICwidhhMZyjFLrk9 o0NWHf+/RwdRTkYTrd5QD+jXPbK7V8xOPpmgWDcDarHytqokdESQUHbg8R5s8V2iBtki IAi0GMwq7gONpHofvc9ILRrS1QHiwRjqvvAtL2LWCiAWbxgnrJeEK6oq5eUXH6bxFTQ7 wEaM5w34CJpvfpvEU3x/WMajp/tdV6rmgBxX0IOwAg0k4iJ/uOBV9naWOWbhur3+2PoN DyLg== X-Gm-Message-State: ALoCoQmsKBuJYW/0VFgOD2usXTLVro50YWtLmpONFa20QQRbbylw8+1M1K1FoeeS13LU7dLt6wfv X-Received: by 10.52.180.3 with SMTP id dk3mr44157311vdc.32.1438103497772; Tue, 28 Jul 2015 10:11:37 -0700 (PDT) MIME-Version: 1.0 Received: by 10.31.191.83 with HTTP; Tue, 28 Jul 2015 10:11:18 -0700 (PDT) From: Caroline Tice Date: Tue, 28 Jul 2015 10:11:18 -0700 Message-ID: Subject: [PATCH, PR 66521] Fix bootstrap segfault with vtable verification enabled To: GCC Patches Cc: Caroline Tice , Luis Lozano X-IsSubscribed: yes I believe the following patch fixes a problem with bootstrap failures on some architectures with vtable verification enabled. The problem was related to a change in name mangling, where classes with anonymous namespaces get "" as their DECL_ASSEMBLER_NAME, rather than the real mangled name. This was causing multiple problems for vtable verification, since not only do we use the mangled name to uniquely identify the various classes (the anonymous classes were no longer being properly 'uniqued'), but also the DECL_ASSEMBLER_NAME was being incorporated into our variable names and ending up in the assembly code and angle-brackets are not legal there. This patch should fix those problems, as well as a few other minor issues I found while working on this. I have bootstrapped with this patch on an x85_64 linux system; I have run all the testsuites with no regressions; and I have verified that it fixes the problem. Is this ok to commit? -- Caroline Tice cmtice@google.com ChangeLogs: libvtv/ChangeLog 2015-07-28 Caroline Tice PR 66521 * Makefile.am: Update to match latest tree. * Makefile.in: Regenerate. * testsuite/lib/libvtv: Brought up to date. * vtv_malloc.cc (VTV_DEBUG): Update function call to match renamed function (old bug!). * vtv_rts.cc (debug_functions, debug_init, debug_verify_vtable): Update initializations to work correctly with VTV_DEBUG defined. gcc/ChangeLog: 2015-07-28 Caroline Tice PR 66521 * vtable-verify.c (vtbl_mangled_name_types, vtbl_mangled_name_ids): New global variables. (vtbl_find_mangled_name): New function. (vtbl_register_mangled_name): New function. (vtbl_map_get_node): If DECL_ASSEMBLER_NAME is "", look up mangled name in mangled name vectors. (find_or_create_vtbl_map_node): Ditto. (var_is_used_for_virtual_call_p): Add recursion_depth parameter; update recursion_depth on function entry; pass it to every recursive call; automatically exit if depth > 25 (give up looking at that point). (verify_bb_vtables): Initialize recursion_depth and pass it to var_is_used_for_virtual_call_p. * vtable-verify.h (vtbl_mangbled_name_types, vtbl_mangled_name_ids): New global variable decls. (vtbl_register_mangled_name): New extern function decl. gcc/cp/ChangeLog: 2015-07-28 Caroline Tice PR 66521 * mangle.c : Add vtable-verify.h to include files. (get_mangled_vtable_map_var_name): If the DECL_ASSEMBLER_NAME is "" get the real mangled name for the class instead, and also store the real mangled name in a vector for use later. Index: gcc/cp/mangle.c =================================================================== --- gcc/cp/mangle.c (revision 226275) +++ gcc/cp/mangle.c (working copy) @@ -62,6 +62,7 @@ #include "function.h" #include "cgraph.h" #include "attribs.h" +#include "vtable-verify.h" /* Debugging support. */ @@ -4034,6 +4035,13 @@ gcc_assert (TREE_CODE (class_type) == RECORD_TYPE); tree class_id = DECL_ASSEMBLER_NAME (TYPE_NAME (class_type)); + + if (strstr (IDENTIFIER_POINTER (class_id), "") != NULL) + { + class_id = get_mangled_id (TYPE_NAME (class_type)); + vtbl_register_mangled_name (TYPE_NAME (class_type), class_id); + } + unsigned int len = strlen (IDENTIFIER_POINTER (class_id)) + strlen (prefix) + strlen (postfix) + 1; Index: gcc/vtable-verify.c =================================================================== --- gcc/vtable-verify.c (revision 226275) +++ gcc/vtable-verify.c (working copy) @@ -310,6 +310,70 @@ /* Vtable map variable nodes stored in a vector. */ vec vtbl_map_nodes_vec; +/* Vector of mangled names for anonymous classes. */ +extern GTY(()) vec *vtbl_mangled_name_types; +extern GTY(()) vec *vtbl_mangled_name_ids; +vec *vtbl_mangled_name_types; +vec *vtbl_mangled_name_ids; + +/* Look up class_type (a type decl for record types) in the vtbl_mangled_names_* + vectors. This is a linear lookup. Return the associated mangled name for + the class type. This is for handling types from anonymous namespaces, whose + DECL_ASSEMBLER_NAME ends up being "", which is useless for our + purposes. + + We use two vectors of trees to keep track of the mangled names: One is a + vector of class types and the other is a vector of the mangled names. The + assumption is that these two vectors are kept in perfect lock-step so that + vtbl_mangled_name_ids[i] is the mangled name for + vtbl_mangled_name_types[i]. */ + +static tree +vtbl_find_mangled_name (tree class_type) +{ + tree result = NULL_TREE; + unsigned i; + + if (!vtbl_mangled_name_types or !vtbl_mangled_name_ids) + return result; + + if (vtbl_mangled_name_types->length() != vtbl_mangled_name_ids->length()) + return result; + + for (i = 0; i < vtbl_mangled_name_types->length(); ++i) + if ((*vtbl_mangled_name_types)[i] == class_type) + { + result = (*vtbl_mangled_name_ids)[i]; + break; + } + + return result; +} + +/* Store a class type decl and its mangled name, for an anonymous RECORD_TYPE, + in the vtbl_mangled_names vector. Make sure there is not already an + entry for the class type before adding it. */ + +void +vtbl_register_mangled_name (tree class_type, tree mangled_name) +{ + if (!vtbl_mangled_name_types) + vec_alloc (vtbl_mangled_name_types, 10); + + if (!vtbl_mangled_name_ids) + vec_alloc (vtbl_mangled_name_ids, 10); + + gcc_assert (vtbl_mangled_name_types->length() == + vtbl_mangled_name_ids->length()); + + + if (vtbl_find_mangled_name (class_type) == NULL_TREE) + { + vec_safe_push (vtbl_mangled_name_types, class_type); + vec_safe_push (vtbl_mangled_name_ids, mangled_name); + } +} + /* Return vtbl_map node for CLASS_NAME without creating a new one. */ struct vtbl_map_node * @@ -339,6 +403,9 @@ gcc_assert (HAS_DECL_ASSEMBLER_NAME_P (class_type_decl)); class_name = DECL_ASSEMBLER_NAME (class_type_decl); + if (strstr (IDENTIFIER_POINTER (class_name), "") != NULL) + class_name = vtbl_find_mangled_name (class_type_decl); + key.class_name = class_name; slot = (struct vtbl_map_node **) vtbl_map_hash->find_slot (&key, NO_INSERT); if (!slot) @@ -370,6 +437,10 @@ gcc_assert (HAS_DECL_ASSEMBLER_NAME_P (class_type_decl)); key.class_name = DECL_ASSEMBLER_NAME (class_type_decl); + + if (strstr (IDENTIFIER_POINTER (key.class_name), "") != NULL) + key.class_name = vtbl_find_mangled_name (class_type_decl); + slot = (struct vtbl_map_node **) vtbl_map_hash->find_slot (&key, INSERT); if (*slot) @@ -482,7 +553,8 @@ the use chain. */ static bool -var_is_used_for_virtual_call_p (tree lhs, int *mem_ref_depth) +var_is_used_for_virtual_call_p (tree lhs, int *mem_ref_depth, + int *recursion_depth) { imm_use_iterator imm_iter; bool found_vcall = false; @@ -494,6 +566,14 @@ if (*mem_ref_depth > 2) return false; + if (*recursion_depth > 25) + /* If we've recursed this far the chances are pretty good that + we're not going to find what we're looking for, and that we've + gone down a recursion black hole. Time to stop. */ + return false; + + *recursion_depth = *recursion_depth + 1; + /* Iterate through the immediate uses of the current variable. If it's a virtual function call, we're done. Otherwise, if there's an LHS for the use stmt, add the ssa var to the work list @@ -516,7 +596,8 @@ { found_vcall = var_is_used_for_virtual_call_p (gimple_phi_result (stmt2), - mem_ref_depth); + mem_ref_depth, + recursion_depth); } else if (is_gimple_assign (stmt2)) { @@ -538,7 +619,8 @@ if (*mem_ref_depth < 3) found_vcall = var_is_used_for_virtual_call_p (gimple_assign_lhs (stmt2), - mem_ref_depth); + mem_ref_depth, + recursion_depth); } else @@ -595,9 +677,11 @@ tree tmp0; bool found; int mem_ref_depth = 0; + int recursion_depth = 0; /* Make sure this vptr field access is for a virtual call. */ - if (!var_is_used_for_virtual_call_p (lhs, &mem_ref_depth)) + if (!var_is_used_for_virtual_call_p (lhs, &mem_ref_depth, + &recursion_depth)) continue; /* Now we have found the virtual method dispatch and Index: gcc/vtable-verify.h =================================================================== --- gcc/vtable-verify.h (revision 226275) +++ gcc/vtable-verify.h (working copy) @@ -127,6 +127,11 @@ /* The global vector of vtbl_map_nodes. */ extern vec vtbl_map_nodes_vec; +/* The global vectors for mangled class names for anonymous classes. */ +extern GTY(()) vec *vtbl_mangled_name_types; +extern GTY(()) vec *vtbl_mangled_name_ids; + +extern void vtbl_register_mangled_name (tree, tree); extern struct vtbl_map_node *vtbl_map_get_node (tree); extern struct vtbl_map_node *find_or_create_vtbl_map_node (tree); extern void vtbl_map_node_class_insert (struct vtbl_map_node *, unsigned); Index: libvtv/testsuite/Makefile.am =================================================================== --- libvtv/testsuite/Makefile.am (revision 226275) +++ libvtv/testsuite/Makefile.am (working copy) @@ -1,11 +1,13 @@ -## Process this with automake to create Makefile.in +## Process this file with automake to produce Makefile.in. AUTOMAKE_OPTIONS = foreign dejagnu -EXPECT = `if [ -f ../../expect/expect ] ; then \ - echo ../../expect/expect ; \ - else echo expect ; fi` +# May be used by various substitution variables. +gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER) -RUNTEST = `if [ -f ${srcdir}/../../dejagnu/runtest ] ; then \ - echo ${srcdir}/../../dejagnu/runtest ; \ - else echo runtest ; fi` +EXPECT = $(shell if test -f $(top_builddir)/../expect/expect; then \ + echo $(top_builddir)/../expect/expect; else echo expect; fi) + +_RUNTEST = $(shell if test -f $(top_srcdir)/../dejagnu/runtest; then \ + echo $(top_srcdir)/../dejagnu/runtest; else echo runtest; fi) +RUNTEST = "$(_RUNTEST) $(AM_RUNTESTFLAGS)" Index: libvtv/testsuite/Makefile.in =================================================================== Regenerated. Index: libvtv/testsuite/lib/libvtv.exp =================================================================== --- libvtv/testsuite/lib/libvtv.exp (revision 226275) +++ libvtv/testsuite/lib/libvtv.exp (working copy) @@ -23,24 +23,28 @@ } load_lib dg.exp + +# Required to use gcc-dg.exp - however, the latter should NOT be +# loaded until ${tool}_target_compile is defined since it uses that +# to determine default LTO options. + +load_gcc_lib prune.exp +load_gcc_lib target-libpath.exp +load_gcc_lib wrapper.exp +load_gcc_lib target-supports.exp +load_gcc_lib target-utils.exp +load_gcc_lib gcc-defs.exp +load_gcc_lib timeout.exp load_gcc_lib file-format.exp -load_gcc_lib target-supports.exp load_gcc_lib target-supports-dg.exp -load_gcc_lib target-utils.exp load_gcc_lib scanasm.exp load_gcc_lib scandump.exp load_gcc_lib scanrtl.exp load_gcc_lib scantree.exp load_gcc_lib scanipa.exp -load_gcc_lib prune.exp -load_gcc_lib target-libpath.exp -load_gcc_lib wrapper.exp -load_gcc_lib gcc-defs.exp +load_gcc_lib timeout-dg.exp load_gcc_lib torture-options.exp -load_gcc_lib timeout.exp -load_gcc_lib timeout-dg.exp load_gcc_lib fortran-modules.exp -load_gcc_lib gcc-dg.exp set dg-do-what-default run @@ -143,10 +147,20 @@ } lappend ALWAYS_CFLAGS "additional_flags=-I${srcdir}/.." + # We use atomic operations in the testcases to validate results. + if { ([istarget i?86-*-*] || [istarget x86_64-*-*]) + && [check_effective_target_ia32] } { + lappend ALWAYS_CFLAGS "additional_flags=-march=i486" + } + if [istarget *-*-darwin*] { lappend ALWAYS_CFLAGS "additional_flags=-shared-libgcc" } + if [istarget sparc*-*-*] { + lappend ALWAYS_CFLAGS "additional_flags=-mcpu=v9" + } + if [info exists TOOL_OPTIONS] { lappend ALWAYS_CFLAGS "additional_flags=$TOOL_OPTIONS" } @@ -155,9 +169,8 @@ # error-message parsing machinery. lappend ALWAYS_CFLAGS "additional_flags=-fmessage-length=0" - # Turn on vtable verification - lappend ALWAYS_CFLAGS "-fvtable-verify=std" - # lappend ALWAYS_CFLAGS "ldflags=-lvtv" + # Turn on vtable verification. + lappend ALWAYS_CFLAGS "additional_flags=-fvtable-verify=std" } # Index: libvtv/vtv_malloc.cc =================================================================== --- libvtv/vtv_malloc.cc (revision 226275) +++ libvtv/vtv_malloc.cc (working copy) @@ -145,7 +145,7 @@ } #ifdef VTV_DEBUG - VTV_malloc_dump_stats (); + __vtv_malloc_dump_stats (); #endif } Index: libvtv/vtv_rts.cc =================================================================== --- libvtv/vtv_rts.cc (revision 226275) +++ libvtv/vtv_rts.cc (working copy) @@ -201,14 +201,15 @@ debugging/tracing will not be ON on production environments */ static const bool debug_hash = HASHTABLE_STATS; -static const int debug_functions = 0; -static const int debug_init = 0; -static const int debug_verify_vtable = 0; #ifdef VTV_DEBUG static const int debug_functions = 1; static const int debug_init = 1; static const int debug_verify_vtable = 1; +#else +static const int debug_functions = 0; +static const int debug_init = 0; +static const int debug_verify_vtable = 0; #endif /* Global file descriptor variables for logging, tracing and debugging. */