From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9218 invoked by alias); 27 Apr 2016 08:49:20 -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 9206 invoked by uid 89); 27 Apr 2016 08:49:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=walked, integral_type_p, INTEGRAL_TYPE_P, type_max_value X-HELO: mx2.suse.de Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Wed, 27 Apr 2016 08:49:08 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 2BE09AB9D for ; Wed, 27 Apr 2016 08:49:02 +0000 (UTC) Date: Wed, 27 Apr 2016 08:49:00 -0000 From: Richard Biener To: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix type field walking in gimplifier unsharing Message-ID: User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-SW-Source: 2016-04/txt/msg01591.txt.bz2 Gimplification is done in-place and thus relies on all processed trees being unshared. This is achieved by unshare_body which in the end uses walk_tree to get at all interesting trees that possibly need unsharing. Unfortunately it doesn't really work because walk_tree only walks types and type-related fields (TYPE_SIZE, TYPE_MIN_VALUE, etc.) in very narrow circumstances. The symptom of failing to unshare trees used in those fields and in the IL of a function body that is gimplified is that those trees get modified by the gimplification process which basically leaks temporary decls into them. Note that only type sizes that are actually needed for the IL of a function are gimplified and thus there are referenced types with still save-expr TYPE_SIZE after gimplification. Eventually dwarf2out knows to emit debug info for those by generating dwarf expressions but will be surely not able to do so if the expressions contain random local decls that may no longer be there. With my patch to make the gimplifier use SSA names for temporaries those may now leak into the non-gimplified TYPE_SIZEs and if they are later released SSA names with NULL_TREE type (or even garbage memory if the SSA name is ggc freed) is in there. This crashes in various ways when those trees are accessed (in dwarf2out, in the LTO streamer, in other places looking at pointed-to types). Thus the following patch which makes the gimplifier unsharing visit all types. Alternative patches include unsharing trees when we build a save_expr around them, doing that only when stor-layout does this to TYPE_SIZE fields (and friends) or try to have an extra pass over the GENERIC IL to just mark the expressions in the type fields not walked by the current walking (might turn out tricky but it would result in the "least" unsharing thus only unshare the parts we eventually gimplify). It might be that we just need to declare tree sharing that runs into the above issue as invalid (they involve Fortran testcases or ubsan testcases only as far as I can see - interestingly no Ada testcases are affected). So - any opinion on the "correct" way to fix this? Clearly the gimplifier running into shared trees is a bug. Thanks, Richard. 2016-04-27 Richard Biener * gimplify.c (copy_if_shared_r): Walk types, type sizes and bounds. (unmark_visited_r): Unmark them. Index: gcc/gimplify.c =================================================================== --- gcc/gimplify.c.orig 2016-04-27 10:29:54.784677194 +0200 +++ gcc/gimplify.c 2016-04-27 10:29:38.708496357 +0200 @@ -832,31 +832,61 @@ copy_if_shared_r (tree *tp, int *walk_su tree t = *tp; enum tree_code code = TREE_CODE (t); - /* Skip types, decls, and constants. But we do want to look at their - types and the bounds of types. Mark them as visited so we properly - unmark their subtrees on the unmark pass. If we've already seen them, - don't look down further. */ - if (TREE_CODE_CLASS (code) == tcc_type - || TREE_CODE_CLASS (code) == tcc_declaration + bool was_visited = TREE_VISITED (t); + + /* If the node wasn't visited already mark it so and recurse on its type. */ + if (! was_visited) + { + TREE_VISITED (t) = 1; + + /* walk_tree does not descend into TREE_TYPE unless this is a + DECL_EXPR of a TYPE_DECL. */ + if (CODE_CONTAINS_STRUCT (code, TS_TYPED)) + walk_tree (&TREE_TYPE (t), copy_if_shared_r, data, NULL); + } + + /* Skip types, decls, and constants for copying. But we do want to look + at their types and the bounds of types. Mark them as visited so we + properly unmark their subtrees on the unmark pass. If we've already + seen them, don't look down further. */ + if (TREE_CODE_CLASS (code) == tcc_declaration || TREE_CODE_CLASS (code) == tcc_constant) { - if (TREE_VISITED (t)) + if (was_visited) + *walk_subtrees = 0; + } + else if (TREE_CODE_CLASS (code) == tcc_type) + { + if (was_visited) *walk_subtrees = 0; else - TREE_VISITED (t) = 1; + { + /* walk_type_fields does not walk type sizes or bounds if not + coming directly from a DECL_EXPR context. */ + /* ??? ideally we'd mark all exprs reached via types as visited + before copy_if_shared so duplicates in types that do not matter + for gimplification are never unshared. */ + walk_tree (&TYPE_SIZE (t), copy_if_shared_r, data, NULL); + walk_tree (&TYPE_SIZE_UNIT (t), copy_if_shared_r, data, NULL); + if (INTEGRAL_TYPE_P (t) + || TREE_CODE (t) == FIXED_POINT_TYPE + || TREE_CODE (t) == REAL_TYPE) + { + walk_tree (&TYPE_MAX_VALUE (t), copy_if_shared_r, data, NULL); + walk_tree (&TYPE_MIN_VALUE (t), copy_if_shared_r, data, NULL); + } + } } /* If this node has been visited already, unshare it and don't look any deeper. */ - else if (TREE_VISITED (t)) + else if (was_visited) { walk_tree (tp, mostly_copy_tree_r, data, NULL); *walk_subtrees = 0; } - /* Otherwise, mark the node as visited and keep looking. */ - else - TREE_VISITED (t) = 1; + /* Otherwise keep looking. */ return NULL_TREE; } @@ -903,7 +933,25 @@ unmark_visited_r (tree *tp, int *walk_su /* If this node has been visited, unmark it and keep looking. */ if (TREE_VISITED (t)) - TREE_VISITED (t) = 0; + { + TREE_VISITED (t) = 0; + + if (TYPE_P (t)) + { + walk_tree (&TYPE_SIZE (t), unmark_visited_r, NULL, NULL); + walk_tree (&TYPE_SIZE_UNIT (t), unmark_visited_r, NULL, NULL); + if (INTEGRAL_TYPE_P (t) + || TREE_CODE (t) == FIXED_POINT_TYPE + || TREE_CODE (t) == REAL_TYPE) + { + walk_tree (&TYPE_MAX_VALUE (t), unmark_visited_r, NULL, NULL); + walk_tree (&TYPE_MIN_VALUE (t), unmark_visited_r, NULL, NULL); + } + } + + if (CODE_CONTAINS_STRUCT (TREE_CODE (t), TS_TYPED)) + walk_tree (&TREE_TYPE (t), unmark_visited_r, NULL, NULL); + } /* Otherwise, don't look any deeper. */ else