From patchwork Sun Aug 19 19:44:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 959437 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-483937-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=hotmail.de Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="mRmaQwwP"; dkim-atps=neutral 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 41tnSl4HCpz9s5c for ; Mon, 20 Aug 2018 05:44:21 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:content-type:mime-version; q=dns; s= default; b=Kat1LxM5w0M/P3Rzs0dlReBtPWaD+erjLoLjsu1i9YJjmiHMGf5i4 y12Cr/I1XgVvH2mA7LwCGeBb41jEHA9zhvQwZ8z4Q40MuhPAenFFeA8wvbW4IoWB SfpKVat/efdEOiSEEARrBHoBfldy0O26jUQZlGGsIsOMsXPenk8Dtw= 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:from :to:subject:date:message-id:content-type:mime-version; s= default; bh=CezTb64nG4wl8NYvocoFimrjpkE=; b=mRmaQwwPJxTZ9wNBTiCd /dw7iYEBSly87UStSNojunS+yFgGecUYpXchyhVHpgyd0nyQ+H9+eVfPIt/GoQ8p Q8FUmhZRNXAO66GN/maqFSRMXbcjw7uBHA9P+BPripvPx3v5QaV8YCnnMmkqmLwa 82fEBxrtgsVljJMEN/4fV58= Received: (qmail 129873 invoked by alias); 19 Aug 2018 19:44:13 -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 129841 invoked by uid 89); 19 Aug 2018 19:44:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.3 required=5.0 tests=AWL, BAYES_50, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=HTo:U*rth, H*Ad:U*rth, RUNTESTFLAGS, runtestflags X-HELO: EUR01-HE1-obe.outbound.protection.outlook.com Received: from mail-oln040092065060.outbound.protection.outlook.com (HELO EUR01-HE1-obe.outbound.protection.outlook.com) (40.92.65.60) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 19 Aug 2018 19:44:10 +0000 Received: from DB5EUR01FT041.eop-EUR01.prod.protection.outlook.com (10.152.4.51) by DB5EUR01HT238.eop-EUR01.prod.protection.outlook.com (10.152.5.206) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.20.1080.9; Sun, 19 Aug 2018 19:44:06 +0000 Received: from AM5PR0701MB2657.eurprd07.prod.outlook.com (10.152.4.54) by DB5EUR01FT041.mail.protection.outlook.com (10.152.5.191) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.20.1080.9 via Frontend Transport; Sun, 19 Aug 2018 19:44:06 +0000 Received: from AM5PR0701MB2657.eurprd07.prod.outlook.com ([fe80::24cf:823c:758c:41b7]) by AM5PR0701MB2657.eurprd07.prod.outlook.com ([fe80::24cf:823c:758c:41b7%7]) with mapi id 15.20.1080.010; Sun, 19 Aug 2018 19:44:06 +0000 From: Bernd Edlinger To: "gcc-patches@gcc.gnu.org" , Richard Henderson , Jeff Law , Jakub Jelinek , Richard Biener Subject: [PATCH] Avoid negative bitpos in expand_expr_real_1 (PR 86984) Date: Sun, 19 Aug 2018 19:44:06 +0000 Message-ID: received-spf: None (protection.outlook.com: hotmail.de does not designate permitted sender hosts) authentication-results: spf=none (sender IP is ) smtp.mailfrom=bernd.edlinger@hotmail.de; MIME-Version: 1.0 Hi! This fixes a wrong code issue in expand_expr_real_1 which happens because a negative bitpos is actually able to reach extract_bit_field which does all computations with poly_uint64, thus the offset 0x1ffffffffffffff0. To avoid that I propose to use Jakub's r204444 patch from the expand_assignment also in the expand_expr_real_1. This is a rather unlikely thing to happen, as there are lots of checks that are of course all target dependent between the get_inner_reference and the actual extract_bit_field call, and all other code paths may or may not have a problem with negative bit offsets. Most don't have a problem, but the bitpos needs to be folded into offset before it is used, therefore it is necessary to handle the negative bitpos very far away from the extract_bit_field call. Doing that later is IMHO not possible. The fix in CONSTANT_ADDRESS_P is actually unrelated, and I only spotted it because this macro is used in alpha_legitimize_address which is of course what one looks at first if something like that happens. I think even with this bogus offset it should not have caused a linker error, so there is probably a second problem in the *movdi code pattern of the alpha.md, because it should be split into instructions that don't cause a link error. Once the target is fixed to split the impossible assembler instruction, the test case will probably no longer be able to detect the pattern in the assembly. Therefore the test case looks both at the assembler output and the expand rtl dump to spot the bogus offset. I only check the first 12 digits of the bogus constant, because it is actually dependent on the target configuration: I built first a cross-compiler without binutils, and it did used a slightly different offset of 0x2000000000000000, (configured with: --target=alpha-linux-gnu --enable-languages=c --disable-libgcc --disable-libssp --disable-libquadmath --disable-libgomp --disable-libatomic) when the binutils are installed at where --prefix points, the offset is 0x1ffffffffffffff0. Regarding the alpha target, I could not do more than build a cross compiler and run make check-gcc-c RUNTESTFLAGS="alpha.exp=pr86984.c". Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. 2018-08-19 Bernd Edlinger PR target/86984 * expr.c (expand_assignment): Assert that bitpos is positive. (store_field): Likewise (expand_expr_real_1): Make sure that bitpos is positive. * config/alpha/alpha.h (CONSTANT_ADDRESS_P): Avoid signed integer overflow. testsuite: 2018-08-19 Bernd Edlinger PR target/86984 * gcc.target/alpha/pr86984.c: New test. Index: gcc/config/alpha/alpha.h =================================================================== --- gcc/config/alpha/alpha.h (revision 263644) +++ gcc/config/alpha/alpha.h (working copy) @@ -678,7 +678,7 @@ enum reg_class { #define CONSTANT_ADDRESS_P(X) \ (CONST_INT_P (X) \ - && (unsigned HOST_WIDE_INT) (INTVAL (X) + 0x8000) < 0x10000) + && (UINTVAL (X) + 0x8000) < 0x10000) /* The macros REG_OK_FOR..._P assume that the arg is a REG rtx and check its validity for a certain class. Index: gcc/expr.c =================================================================== --- gcc/expr.c (revision 263644) +++ gcc/expr.c (working copy) @@ -5270,6 +5270,7 @@ expand_assignment (tree to, tree from, bool nontem MEM_VOLATILE_P (to_rtx) = 1; } + gcc_assert (known_ge (bitpos, 0)); if (optimize_bitfield_assignment_op (bitsize, bitpos, bitregion_start, bitregion_end, mode1, to_rtx, to, from, @@ -7046,6 +7047,7 @@ store_field (rtx target, poly_int64 bitsize, poly_ } /* Store the value in the bitfield. */ + gcc_assert (known_ge (bitpos, 0)); store_bit_field (target, bitsize, bitpos, bitregion_start, bitregion_end, mode, temp, reverse); @@ -10545,6 +10547,14 @@ expand_expr_real_1 (tree exp, rtx target, machine_ mode2 = CONSTANT_P (op0) ? TYPE_MODE (TREE_TYPE (tem)) : GET_MODE (op0); + /* Make sure bitpos is not negative, it can wreak havoc later. */ + if (maybe_lt (bitpos, 0)) + { + gcc_assert (offset == NULL_TREE); + offset = size_int (bits_to_bytes_round_down (bitpos)); + bitpos = num_trailing_bits (bitpos); + } + /* If we have either an offset, a BLKmode result, or a reference outside the underlying object, we must force it to memory. Such a case can occur in Ada if we have unchecked conversion @@ -10795,6 +10805,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_ && GET_MODE_CLASS (ext_mode) == MODE_INT) reversep = TYPE_REVERSE_STORAGE_ORDER (type); + gcc_assert (known_ge (bitpos, 0)); op0 = extract_bit_field (op0, bitsize, bitpos, unsignedp, (modifier == EXPAND_STACK_PARM ? NULL_RTX : target), Index: gcc/testsuite/gcc.target/alpha/pr86984.c =================================================================== --- gcc/testsuite/gcc.target/alpha/pr86984.c (revision 0) +++ gcc/testsuite/gcc.target/alpha/pr86984.c (working copy) @@ -0,0 +1,96 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall -Wwrite-strings -Werror -fmerge-all-constants -fno-stack-protector -mieee -fdump-rtl-expand" } */ + +struct expression { + unsigned long int num; +}; +union YYSTYPE { + unsigned long int num; + struct expression *exp; +}; + +typedef union YYSTYPE YYSTYPE; + +struct expression * new_exp_0 (int); + +union yyalloc { + short yyss_alloc; +}; + +static const signed char yypact[] = { + -9, -9, -10, -10, -9, 8, 36, -10, 13, -10, -9, -9, -9, -9, -9, -9, -9, -10, 26, 41, 45, 18, -2, 14, -10, -9, 36 }; +static const unsigned char yydefact[] = { + 0, 0, 12, 11, 0, 0, 2, 10, 0, 1, 0, 0, 0, 0, 0, 0, 0, 13, 0, 4, 5, 6, 7, 8, 9, 0, 3 }; + +static const signed char yypgoto[3] = "\366\366\377"; +static const signed char yydefgoto[3] = "\377\005\006"; + +static const unsigned char yytable[] = { + 7, 1, 2, 8, 3, 4, 15, 16, 9, 18, 19, 20, 21, 22, 23, 24, 10, 11, 12, 13, 14, 15, 16, 16, 26, 14, 15, 16, 17, 10, 11, 12, 13, 14, 15, 16, 0, 0, 25, 10, 11, 12, 13, 14, 15, 16, 12, 13, 14, 15, 16, 13, 14, 15, 16 }; + +static const signed char yycheck[] = { + 1, 10, 11, 4, 13, 14, 8, 9, 0, 10, 11, 12, 13, 14, 15, 16, 3, 4, 5, 6, 7, 8, 9, 9, 25, 7, 8, 9, 15, 3, 4, 5, 6, 7, 8, 9, -1, -1, 12, 3, 4, 5, 6, 7, 8, 9, 5, 6, 7, 8, 9, 6, 7, 8, 9 }; + +static const unsigned char yyr1[] = { + 0, 16, 17, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18 }; + +static const unsigned char yyr2[] = { + 0, 2, 1, 5, 3, 3, 3, 3, 3, 3, 2, 1, 1, 3 }; + +int __gettextparse (void) +{ + int yystate = 0; + short yyssa[200]; + short *yyss = yyss; + short *yyssp = yyssa; + YYSTYPE yyvsa[200]; + YYSTYPE *yyvsp = yyvsa; + enum { yystacksize = 200 }; + int yylen = 0; + goto yysetstate; + yynewstate: yyssp++; + yysetstate: *yyssp = yystate; + + if (yyss + yystacksize - 1 <= yyssp) + { + long unsigned int yysize = yyssp - yyss + 1; + { + short *yyss1 = yyss; + union yyalloc *yyptr = (union yyalloc *) __builtin_malloc ((yystacksize * (sizeof (short) + sizeof (YYSTYPE)) + (sizeof (union yyalloc) - 1))); + if (!yyptr) return 0; + __builtin_memcpy (&yyptr->yyss_alloc, yyss, yysize * sizeof *(yyss)); + yyss = &yyptr->yyss_alloc; + if (yyss1 != yyssa) __builtin_free (yyss1); + } + if (yyss + yystacksize - 1 <= yyssp) + return 0; + } + + int yyn = yypact[yystate]; + if (yyn == -10) + goto yydefault; + + yyn = yytable[yyn]; + if (yyn <= 0) + goto yyreduce; + + yydefault: yyn = yydefact[yystate]; + yyreduce: yylen = yyr2[yyn]; + + YYSTYPE yyval; + if (yyn == 12 && (yyval.exp = new_exp_0 (0)) != 0) + (yyval.exp)->num = (yyvsp[0].num); + + (yyvsp -= yylen, yyssp -= yylen); + yyn = yyr1[yyn]; + yystate = yypgoto[yyn - 16] + *yyssp; + if (0 <= yystate && yystate <= 54 && yycheck[yystate] == *yyssp) + yystate = yytable[yystate]; + else + yystate = yydefgoto[yyn - 16]; + + goto yynewstate; +} + +/* { dg-final { scan-rtl-dump-not "const_int 230584300921" "expand" } } */ +/* { dg-final { scan-assembler-not "yypgoto\\+230584300921" } } */