From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38226 invoked by alias); 20 Feb 2018 17:49:46 -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 38117 invoked by uid 89); 20 Feb 2018 17:49:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-12.2 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_LOW,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=UD:builtin-sprintf-warn-3.c, UD:builtin-sprintf-warn-11.c, distilled X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Feb 2018 17:49:43 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 502004040857 for ; Tue, 20 Feb 2018 17:49:30 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-204-85.brq.redhat.com [10.40.204.85]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 80D2D2026E04 for ; Tue, 20 Feb 2018 17:49:29 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id w1KHHDPg015787; Tue, 20 Feb 2018 18:17:14 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id w1KHHCtF013452; Tue, 20 Feb 2018 18:17:12 +0100 Date: Tue, 20 Feb 2018 17:49:00 -0000 From: Jakub Jelinek To: Richard Biener , Jeff Law Cc: gcc-patches@gcc.gnu.org, Martin Sebor Subject: [PATCH] Fix pdftex miscompilation due to get_range_strlen (PR tree-optimization/84478) Message-ID: <20180220171712.GH5867@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.9.1 (2017-09-22) X-IsSubscribed: yes X-SW-Source: 2018-02/txt/msg01178.txt.bz2 Hi! The following testcase distilled from pdftex is miscompiled on i?86, because gimple_fold_builtin_strlen sets incorrect value range on strlen call on SSA_NAME with def_stmt of PHI <"mu", something> where we can't determine anything about string length of something, so the right value range is [0, maximum_possible_strlen], but we actually used [2, INF]. get_range_strlen has many modes, for get_maxval_strlen we check return value of get_range_strlen, don't set fuzzy and everything seems correct. Otherwise we have the fuzzy mode which is used in 2 arg get_range_strlen overload, which is used for warnings and recently also for gimple_fold_builtin_strlen which sets value ranges from it. Unlike warnings, which can perhaps afford some heuristics and guessing, for gimple_fold_builtin_strlen we need to be 100% conservative. The thing that isn't handled conservatively is PHIs and COND_EXPR. The current code, if we can't figure one of the args out, for PHIs in fuzzy mode increases the *maxval value to +INF, but doesn't touch *minval, for COND_EXPR doesn't adjust the *minval/*maxval at all and just returns false. Unfortunately, changing that breaks FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 165) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 166) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 167) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 168) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 309) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 310) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 311) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for warnings, line 312) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-11.c (test for excess errors) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 62) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 63) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 402) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 403) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 430) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 431) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 458) FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-3.c (test for warnings, line 459) so this patch instead stops using the 2 argument get_range_strlen in gimple_fold_builtin_strlen and makes sure we handle that case conservatively, i.e. PHI of something known + unknown is unknown, likewise COND_EXPR. Bootstrapped on x86_64-linux and i686-linux, regtest pending, ok for trunk if it passes regtest on both? For stage1 I guess Martin can tweak the warning code paths and if they will become the same as the conservatively correct ones, what is in this patch can be adjusted again. 2018-02-20 Jakub Jelinek PR tree-optimization/84478 * gimple-fold.c (get_range_strlen): Make minlen const and assume it can't be NULL. Add type 3 support which is conservatively correct in PHIs. Formatting and comment capitalization fixes. Add warning that the 2 argument get_range_strlen is only usable for warnings. (gimple_fold_builtin_strlen): Use the 6 arg get_range_strlen overload rather than 2 arg, use it only if it returns true and flexarray is false, pass 3 as type to it. * gcc.c-torture/execute/pr84478.c: New test. --- gcc/gimple-fold.c.jj 2018-02-19 19:57:03.424279589 +0100 +++ gcc/gimple-fold.c 2018-02-20 16:21:34.583020305 +0100 @@ -1283,10 +1283,11 @@ gimple_fold_builtin_memset (gimple_stmt_ value of ARG in LENGTH[0] and LENGTH[1], respectively. If ARG is an SSA name variable, follow its use-def chains. When TYPE == 0, if LENGTH[1] is not equal to the length we determine or - if we are unable to determine the length or value, return False. + if we are unable to determine the length or value, return false. VISITED is a bitmap of visited variables. TYPE is 0 if string length should be obtained, 1 for maximum string - length and 2 for maximum value ARG can have. + length and 2 for maximum value ARG can have, 3 is like 1, but provide + conservatively correct rather than optimistic answer. When FUZZY is set and the length of a string cannot be determined, the function instead considers as the maximum possible length the size of a character array it may refer to. @@ -1302,9 +1303,8 @@ get_range_strlen (tree arg, tree length[ tree var, val = NULL_TREE; gimple *def_stmt; - /* The minimum and maximum length. The MAXLEN pointer stays unchanged - but MINLEN may be cleared during the execution of the function. */ - tree *minlen = length; + /* The minimum and maximum length. */ + tree *const minlen = length; tree *const maxlen = length + 1; if (TREE_CODE (arg) != SSA_NAME) @@ -1445,12 +1445,11 @@ get_range_strlen (tree arg, tree length[ if (!val) return false; - if (minlen - && (!*minlen - || (type > 0 - && TREE_CODE (*minlen) == INTEGER_CST - && TREE_CODE (val) == INTEGER_CST - && tree_int_cst_lt (val, *minlen)))) + if (!*minlen + || (type > 0 + && TREE_CODE (*minlen) == INTEGER_CST + && TREE_CODE (val) == INTEGER_CST + && tree_int_cst_lt (val, *minlen))) *minlen = val; if (*maxlen) @@ -1503,18 +1502,16 @@ get_range_strlen (tree arg, tree length[ { tree op2 = gimple_assign_rhs2 (def_stmt); tree op3 = gimple_assign_rhs3 (def_stmt); - return get_range_strlen (op2, length, visited, type, fuzzy, flexp) - && get_range_strlen (op3, length, visited, type, fuzzy, flexp); + return (get_range_strlen (op2, length, visited, type, fuzzy, flexp) + && get_range_strlen (op3, length, visited, type, fuzzy, + flexp)); } return false; case GIMPLE_PHI: - { - /* All the arguments of the PHI node must have the same constant - length. */ - unsigned i; - - for (i = 0; i < gimple_phi_num_args (def_stmt); i++) + /* All the arguments of the PHI node must have the same constant + length. */ + for (unsigned i = 0; i < gimple_phi_num_args (def_stmt); i++) { tree arg = gimple_phi_arg (def_stmt, i)->def; @@ -1529,13 +1526,12 @@ get_range_strlen (tree arg, tree length[ if (!get_range_strlen (arg, length, visited, type, fuzzy, flexp)) { - if (fuzzy) + if (fuzzy && type != 3) *maxlen = build_all_ones_cst (size_type_node); else return false; } } - } return true; default: @@ -1554,7 +1550,10 @@ get_range_strlen (tree arg, tree length[ Return true if the range of the string lengths has been obtained from the upper bound of an array at the end of a struct. Such an array may hold a string that's longer than its upper bound - due to it being used as a poor-man's flexible array member. */ + due to it being used as a poor-man's flexible array member. + + This function should be only used for warning code, as it doesn't + handle PHIs in a conservatively correct way. */ bool get_range_strlen (tree arg, tree minmaxlen[2]) @@ -3533,8 +3532,12 @@ gimple_fold_builtin_strlen (gimple_stmt_ wide_int minlen; wide_int maxlen; - tree lenrange[2]; - if (!get_range_strlen (gimple_call_arg (stmt, 0), lenrange) + tree lenrange[2] = { NULL_TREE, NULL_TREE }; + bitmap visited = NULL; + bool flexarray = false; + if (get_range_strlen (gimple_call_arg (stmt, 0), lenrange, &visited, + 3, true, &flexarray) + && !flexarray && lenrange[0] && TREE_CODE (lenrange[0]) == INTEGER_CST && lenrange[1] && TREE_CODE (lenrange[1]) == INTEGER_CST) { @@ -3554,6 +3557,9 @@ gimple_fold_builtin_strlen (gimple_stmt_ maxlen = wi::to_wide (max_object_size (), prec) - 2; } + if (visited) + BITMAP_FREE (visited); + if (minlen == maxlen) { lenrange[0] = force_gimple_operand_gsi (gsi, lenrange[0], true, NULL, --- gcc/testsuite/gcc.c-torture/execute/pr84478.c.jj 2018-02-20 16:32:00.683086212 +0100 +++ gcc/testsuite/gcc.c-torture/execute/pr84478.c 2018-02-20 16:31:33.497081640 +0100 @@ -0,0 +1,49 @@ +/* PR tree-optimization/84478 */ + +long poolptr; +unsigned char *strpool; +static const char *poolfilearr[] = { + "mu", + "", +#define A "x", +#define B A "xx", A A "xxx", A A A A A +#define C B B B B B B B B B B +#define D C C C C C C C C C C + D C C C C C C C B B B + ((void *)0) +}; + +__attribute__((noipa)) long +makestring (void) +{ + return 1; +} + +__attribute__((noipa)) long +loadpoolstrings (long spare_size) +{ + const char *s; + long g = 0; + int i = 0, j = 0; + while ((s = poolfilearr[j++])) + { + int l = __builtin_strlen (s); + i += l; + if (i >= spare_size) return 0; + while (l-- > 0) strpool[poolptr++] = *s++; + g = makestring (); + } + return g; +} + +int +main () +{ + strpool = __builtin_malloc (4000); + if (!strpool) + return 0; + asm volatile ("" : : : "memory"); + volatile int r = loadpoolstrings (4000); + __builtin_free (strpool); + return 0; +} Jakub