From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67948 invoked by alias); 8 Oct 2016 18:56:39 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 67933 invoked by uid 89); 8 Oct 2016 18:56:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,FILL_THIS_FORM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=no version=3.3.2 spammy=PLUS_EXPR, plus_expr, undesirable, fallout X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 08 Oct 2016 18:56:27 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 90AD381329 for ; Sat, 8 Oct 2016 20:56:25 +0200 (CEST) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id R6T7kRcp5chI for ; Sat, 8 Oct 2016 20:56:25 +0200 (CEST) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 5CD8581307 for ; Sat, 8 Oct 2016 20:56:25 +0200 (CEST) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [patch] Fix GC issue triggered by arithmetic overflow checking Date: Sat, 08 Oct 2016 18:56:00 -0000 Message-ID: <1863165.r8qPLI7fxq@polaris> User-Agent: KMail/4.14.10 (Linux/3.16.7-42-desktop; KDE/4.14.9; x86_64; ; ) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart23860093.FvAGkWclMG" Content-Transfer-Encoding: 7Bit X-SW-Source: 2016-10/txt/msg00541.txt.bz2 This is a multi-part message in MIME format. --nextPart23860093.FvAGkWclMG Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Content-length: 3454 Hi, adding patterns for unsigned arithmetic overflow checking in a back-end can have unexpected fallout because of a latent GC issue: when they are present, GIMPLE optimization passes can create complex (math. sense) types at will by invoking build_complex_type. Now build_complex_type goes through the type caonicalization hashtable, which is GC-ed, so its behavior depends on the actual collection points. The other type-building functions present in tree.c do the same so no big deal but build_complex_type is special because it also does: /* We need to create a name, since complex is a fundamental type. */ if (! TYPE_NAME (t)) { const char *name; if (component_type == char_type_node) name = "complex char"; else if (component_type == signed_char_type_node) name = "complex signed char"; else if (component_type == unsigned_char_type_node) name = "complex unsigned char"; else if (component_type == short_integer_type_node) name = "complex short int"; else if (component_type == short_unsigned_type_node) name = "complex short unsigned int"; else if (component_type == integer_type_node) name = "complex int"; else if (component_type == unsigned_type_node) name = "complex unsigned int"; else if (component_type == long_integer_type_node) name = "complex long int"; else if (component_type == long_unsigned_type_node) name = "complex long unsigned int"; else if (component_type == long_long_integer_type_node) name = "complex long long int"; else if (component_type == long_long_unsigned_type_node) name = "complex long long unsigned int"; else name = 0; if (name != 0) TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL, get_identifier (name), t); } so it creates a DECL node every time a new canonical complex type is created, bumping the DECL_UID counter in the process. Which means that the DECL_UID counter is sensitive to the collection points, which in turn means that the result of algorithms depending on the DECL_UID counter also is. This for example resulted in a bootstrap comparison failure on a SPARC/Solaris machine doing a strict stage2/stage3 comparison because the contents of the .debug_loc section were different: location lists computed by var-tracking were slightly different because of a different hashing. I'm not sure whether the hashing done by var-tracking should be sensitive to the DECL_UID of nodes or not, but I think that having the DECL_UID counter depend on the collection points is highly undesirable, so the attached patch attempts to prevent it; it at least fixed the bootstrap comparison failure. Tested on x86_64-suse-linux, OK for the mainline? 2016-10-08 Eric Botcazou * tree.h (build_complex_type): Add second parameter with default. * builtins.c (expand_builtin_cexpi): Pass false in call to above. (fold_builtin_sincos): Likewise. (fold_builtin_arith_overflow): Likewise. * gimple-fold.c (fold_builtin_atomic_compare_exchange): Likewise. (gimple_fold_call): Likewise. * stor-layout.c (bitwise_type_for_mode): Likewise. * tree-ssa-dce.c (maybe_optimize_arith_overflow): Likewise. * tree-ssa-math-opts.c (match_uaddsub_overflow): Likewise. * tree.c (build_complex): Likewise. (build_complex_type): Add NAMED second parameter and adjust recursive call. Create a TYPE_DECL only if NAMED is true. -- Eric Botcazou --nextPart23860093.FvAGkWclMG Content-Disposition: attachment; filename="p.diff" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="p.diff" Content-length: 6426 Index: builtins.c =================================================================== --- builtins.c (revision 240888) +++ builtins.c (working copy) @@ -2356,7 +2356,7 @@ expand_builtin_cexpi (tree exp, rtx targ else { tree call, fn = NULL_TREE, narg; - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CEXPIF) fn = builtin_decl_explicit (BUILT_IN_CEXPF); @@ -2396,7 +2396,7 @@ expand_builtin_cexpi (tree exp, rtx targ } /* Now build the proper return type. */ - return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type), + return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type, false), make_tree (TREE_TYPE (arg), op2), make_tree (TREE_TYPE (arg), op1)), target, VOIDmode, EXPAND_NORMAL); @@ -7226,7 +7226,7 @@ fold_builtin_sincos (location_t loc, /* Canonicalize sincos to cexpi. */ if (TREE_CODE (arg0) == REAL_CST) { - tree complex_type = build_complex_type (type); + tree complex_type = build_complex_type (type, false); call = fold_const_call (as_combined_fn (fn), complex_type, arg0); } if (!call) @@ -8137,7 +8137,7 @@ fold_builtin_arith_overflow (location_t ? boolean_true_node : boolean_false_node, arg2); - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); tree call = build_call_expr_internal_loc (loc, ifn, ctype, 2, arg0, arg1); tree tgt = save_expr (call); Index: gimple-fold.c =================================================================== --- gimple-fold.c (revision 240888) +++ gimple-fold.c (working copy) @@ -3210,7 +3210,7 @@ fold_builtin_atomic_compare_exchange (gi tree fndecl = gimple_call_fndecl (stmt); tree parmt = TYPE_ARG_TYPES (TREE_TYPE (fndecl)); tree itype = TREE_VALUE (TREE_CHAIN (TREE_CHAIN (parmt))); - tree ctype = build_complex_type (itype); + tree ctype = build_complex_type (itype, false); tree expected = TREE_OPERAND (gimple_call_arg (stmt, 1), 0); gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (expected)), expected); @@ -3582,7 +3582,7 @@ gimple_fold_call (gimple_stmt_iterator * { if (overflow == NULL_TREE) overflow = build_zero_cst (TREE_TYPE (result)); - tree ctype = build_complex_type (TREE_TYPE (result)); + tree ctype = build_complex_type (TREE_TYPE (result), false); if (TREE_CODE (result) == INTEGER_CST && TREE_CODE (overflow) == INTEGER_CST) result = build_complex (ctype, result, overflow); Index: stor-layout.c =================================================================== --- stor-layout.c (revision 240888) +++ stor-layout.c (working copy) @@ -469,7 +469,7 @@ bitwise_type_for_mode (machine_mode mode return build_vector_type_for_mode (inner_type, mode); if (COMPLEX_MODE_P (mode)) - return build_complex_type (inner_type); + return build_complex_type (inner_type, false); gcc_checking_assert (GET_MODE_INNER (mode) == mode); return inner_type; Index: tree-ssa-dce.c =================================================================== --- tree-ssa-dce.c (revision 240888) +++ tree-ssa-dce.c (working copy) @@ -1194,7 +1194,7 @@ maybe_optimize_arith_overflow (gimple_st if (TREE_CODE (result) == INTEGER_CST && TREE_OVERFLOW (result)) result = drop_tree_overflow (result); tree overflow = build_zero_cst (type); - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); if (TREE_CODE (result) == INTEGER_CST) result = build_complex (ctype, result, overflow); else Index: tree-ssa-math-opts.c =================================================================== --- tree-ssa-math-opts.c (revision 240888) +++ tree-ssa-math-opts.c (working copy) @@ -3736,7 +3736,7 @@ match_uaddsub_overflow (gimple_stmt_iter if (!ovf_use_seen || !use_seen) return false; - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); tree rhs1 = gimple_assign_rhs1 (stmt); tree rhs2 = gimple_assign_rhs2 (stmt); gcall *g = gimple_build_call_internal (code == PLUS_EXPR Index: tree.c =================================================================== --- tree.c (revision 240888) +++ tree.c (working copy) @@ -2021,7 +2021,7 @@ build_complex (tree type, tree real, tre TREE_REALPART (t) = real; TREE_IMAGPART (t) = imag; - TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real)); + TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real), false); TREE_OVERFLOW (t) = TREE_OVERFLOW (real) | TREE_OVERFLOW (imag); return t; } @@ -8758,10 +8758,15 @@ build_offset_type (tree basetype, tree t return t; } -/* Create a complex type whose components are COMPONENT_TYPE. */ +/* Create a complex type whose components are COMPONENT_TYPE. + + If NAMED is true, the type is given a TYPE_NAME. We do not always + do so because this creates a DECL node and thus make the DECL_UIDs + dependent on the type canonicalization hashtable, which is GC-ed, + so the DECL_UIDs would not be stable wrt garbage collection. */ tree -build_complex_type (tree component_type) +build_complex_type (tree component_type, bool named) { tree t; inchash::hash hstate; @@ -8788,11 +8793,11 @@ build_complex_type (tree component_type) SET_TYPE_STRUCTURAL_EQUALITY (t); else if (TYPE_CANONICAL (component_type) != component_type) TYPE_CANONICAL (t) - = build_complex_type (TYPE_CANONICAL (component_type)); + = build_complex_type (TYPE_CANONICAL (component_type), named); } /* We need to create a name, since complex is a fundamental type. */ - if (! TYPE_NAME (t)) + if (!TYPE_NAME (t) && named) { const char *name; if (component_type == char_type_node) Index: tree.h =================================================================== --- tree.h (revision 240888) +++ tree.h (working copy) @@ -4042,7 +4042,7 @@ extern tree build_varargs_function_type_ extern tree build_method_type_directly (tree, tree, tree); extern tree build_method_type (tree, tree); extern tree build_offset_type (tree, tree); -extern tree build_complex_type (tree); +extern tree build_complex_type (tree, bool named = true); extern tree array_type_nelts (const_tree); extern tree value_member (tree, tree); --nextPart23860093.FvAGkWclMG--