From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-DB5-obe.outbound.protection.outlook.com (mail-eopbgr40050.outbound.protection.outlook.com [40.107.4.50]) by server2.sourceware.org (Postfix) with ESMTPS id 72DC03947038; Sat, 7 Mar 2020 21:44:19 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1+Oge2Kd+qcJ+wHnThgccBQF5Ns7O4ZEuy/skvY5R/8=; b=NrEryRsBUPIK0SwEdIeEiYAKkfcIsYrJEdfJO3hkNIhJyW5QdqzLXjJdh7aS6htAFc209UI3mv4iMc0zjMuRliM4+OxaC1E6KI+ff4q6y5KwJ7nOjcLtiSlhW1iVZD6bhsjPVcJ1CDt9cNdYOfo8nJ/ncXHyqG/aLwREREkQ7+k= Received: from VI1PR07CA0210.eurprd07.prod.outlook.com (2603:10a6:802:3f::34) by VI1PR08MB4093.eurprd08.prod.outlook.com (2603:10a6:803:de::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.18; Sat, 7 Mar 2020 21:44:15 +0000 Received: from VE1EUR03FT056.eop-EUR03.prod.protection.outlook.com (2603:10a6:802:3f:cafe::1) by VI1PR07CA0210.outlook.office365.com (2603:10a6:802:3f::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2814.9 via Frontend Transport; Sat, 7 Mar 2020 21:44:15 +0000 Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; gcc.gnu.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;gcc.gnu.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT056.mail.protection.outlook.com (10.152.19.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.11 via Frontend Transport; Sat, 7 Mar 2020 21:44:15 +0000 Received: ("Tessian outbound d1ceabc7047e:v42"); Sat, 07 Mar 2020 21:44:14 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 59b925e92f850c7d X-CR-MTA-TID: 64aa7808 Received: from 0f6f31628265.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id A7A2B92B-FC16-4A44-8BCC-3FE9DAF13966.1; Sat, 07 Mar 2020 21:44:09 +0000 Received: from EUR03-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 0f6f31628265.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Sat, 07 Mar 2020 21:44:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eEnqbCBl+erfw3U2GrbAadplmkhkAZFsm5sbstgkUUvMkmtb6RRNnOy3auA7NG07ZZT1om2+09MgjjDGrE22qKlLHgsPbIAt1rT6no5Os2HP36gl/ZRiYb6XHc4hy51jBwmlQZmaQKkhLx+JWSTCxdapHDJbA9wqhW6NSE4Or+WROc2D6wvcEdo73/DaXav802C3BU4eFeGNvlcJ2RAo3ny4vewf3tSexzREWFLFi1Gk8xSgR+TMTNiuRgEcnSrJouqxpaUBCLQOIPXIIqFS6xgxs53HoTgU3TRlv/VJHHRLzv10zulEb9aWYymXRnfvGYcUw3oJuVrPLjemBxPdRg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1+Oge2Kd+qcJ+wHnThgccBQF5Ns7O4ZEuy/skvY5R/8=; b=fgMXU9Ube9dbV4mOl6cCKe/x+4y6b6roaNGzlHP7BtYgeq0b562zYK13N679uE9mRf1sXEWUMInmMSOk5Fw3eYtE/N4goD42bCWciLluri+Bu9XkuPTYRuQdYcMWUswrGqsDu90ccQaxPfgHqCdaBAhB4vedMQvHj9qcekAwpPCqJltpidCtlXuLyZ4YJyP+oYYh/OEzM36euZ5isbINJx89opS91NORXzWBg5NSBls8L5YTUxxM5qvvkAXv2RIlJ/RFRAZQpizXZMQNbyPoZ7SYakVQP4fwbxUfEuaOG98BMwWxLKOKXuYnuhXNSIYahyTKQaMOwDoHOsIui1gJXw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1+Oge2Kd+qcJ+wHnThgccBQF5Ns7O4ZEuy/skvY5R/8=; b=NrEryRsBUPIK0SwEdIeEiYAKkfcIsYrJEdfJO3hkNIhJyW5QdqzLXjJdh7aS6htAFc209UI3mv4iMc0zjMuRliM4+OxaC1E6KI+ff4q6y5KwJ7nOjcLtiSlhW1iVZD6bhsjPVcJ1CDt9cNdYOfo8nJ/ncXHyqG/aLwREREkQ7+k= Authentication-Results-Original: spf=none (sender IP is ) smtp.mailfrom=Andrea.Corallo@arm.com; Received: from VI1PR08MB2765.eurprd08.prod.outlook.com (10.170.236.32) by VI1PR08MB4560.eurprd08.prod.outlook.com (20.179.24.80) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2772.15; Sat, 7 Mar 2020 21:44:07 +0000 Received: from VI1PR08MB2765.eurprd08.prod.outlook.com ([fe80::eca9:5f98:627d:571b]) by VI1PR08MB2765.eurprd08.prod.outlook.com ([fe80::eca9:5f98:627d:571b%6]) with mapi id 15.20.2772.019; Sat, 7 Mar 2020 21:44:07 +0000 From: Andrea Corallo To: David Malcolm Cc: "jit@gcc.gnu.org" , "gcc-patches@gcc.gnu.org" , nd Subject: Re: [PATCH][gcc] libgccjit: handle long literals in playback::context::new_string_literal References: Date: Sat, 07 Mar 2020 21:43:58 +0000 In-Reply-To: (David Malcolm's message of "Thu, 05 Mar 2020 22:18:46 -0500") Message-ID: <878skb263l.fsf@arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Content-Type: text/plain X-ClientProxiedBy: SN1PR12CA0057.namprd12.prod.outlook.com (2603:10b6:802:20::28) To VI1PR08MB2765.eurprd08.prod.outlook.com (2603:10a6:802:18::32) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from krcb (217.140.106.52) by SN1PR12CA0057.namprd12.prod.outlook.com (2603:10b6:802:20::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.16 via Frontend Transport; Sat, 7 Mar 2020 21:44:05 +0000 X-Originating-IP: [217.140.106.52] X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: d92f88b2-613b-43fe-8266-08d7c2e0aea9 X-MS-TrafficTypeDiagnostic: VI1PR08MB4560:|VI1PR08MB4093: X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:10000;OLM:10000; X-Forefront-PRVS: 03355EE97E X-Forefront-Antispam-Report-Untrusted: SFV:NSPM; SFS:(10009020)(4636009)(366004)(39850400004)(346002)(396003)(376002)(136003)(189003)(199004)(5660300002)(186003)(16526019)(52116002)(44832011)(36756003)(6666004)(2906002)(86362001)(6496006)(956004)(6486002)(81166006)(66946007)(4326008)(478600001)(8936002)(316002)(66556008)(8676002)(54906003)(26005)(66476007)(81156014)(2616005)(6916009); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR08MB4560; H:VI1PR08MB2765.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: +uVVeJRHuoBUJ2/PVQSC0N6p95GigBBNgc5nC9n1EltBlPW6CdWjvIhNpNyP83kWvYxQQb1Apzf9goGYN6D8tDfn9AtTnVKjQ4s2LSwa+RtOEeecNyNGPlq5xt4hot47WQmw5y4Sg7hi4ksYPKGbK9jft3rjlyCTYQQ8wgUN5hMurozN2YpIvcbPNh2ZJD34PZA8JRlvzP2+AmWeLJqaiW93CDTXGf7MNM243DsLXQm7WgbuEFe/Idi7FVUW6EIFKHQ7j3kd1kGS00LrAlSty3BoHinP91Pkqkr10bchZP54YxIOH2TmkkOlLMivlKaHvVl47PQRKfjmqahHw2RrhzWKhaogkzBXv8X0tDm2CLGvitqTN4mXDjUYwMIQujMt92YTx9xw4RRSlST1O0Ao0PO7kP0kbE4GyAzYJqSAIABsx1OoNP17a3HLVy///Xb+ X-MS-Exchange-AntiSpam-MessageData: frAio5ZMPUa4/8tFxLVckNgjqHKxT/X/Bbk/JqqOGHVoYhAc4pDIoEO9oOJ1soI+lwTaHQ0bttynEA0O4QvN0kaZAU0Rj6yfo/PEAQz+QB6ExTn/VYwjosNjfyLW2sCER1IpM055N/n7UKVYRMpfxg== X-MS-Exchange-Transport-Forked: True X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB4560 Original-Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Andrea.Corallo@arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT056.eop-EUR03.prod.protection.outlook.com X-Forefront-Antispam-Report: CIP:63.35.35.123; IPV:CAL; SCL:-1; CTRY:IE; EFV:NLI; SFV:NSPM; SFS:(10009020)(4636009)(136003)(39850400004)(376002)(346002)(396003)(199004)(189003)(8936002)(186003)(44832011)(2906002)(26826003)(6496006)(478600001)(5660300002)(956004)(336012)(54906003)(2616005)(26005)(450100002)(70206006)(86362001)(4326008)(81166006)(70586007)(16526019)(81156014)(36906005)(36756003)(8676002)(6862004)(316002)(6666004)(356004)(6486002); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR08MB4093; H:64aa7808-outbound-1.mta.getcheckrecipient.com; FPR:; SPF:Pass; LANG:en; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; A:1; MX:1; X-MS-Office365-Filtering-Correlation-Id-Prvs: e29eda70-42a4-49cb-2a4c-08d7c2e0a974 X-Forefront-PRVS: 03355EE97E X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: tFELI3Kzt+V0DMxwvS+Pk7HHjsOtRfWuuLg5dxbPeWpDlO7ZUrRJxA8tnK7PVtR9i+w51IY6/+zBgKWGyuCgFx92pSBIccgoeuYN1JUikIkUBxjkJx1QwZN8E1qjSO2iGkE56iPmMPYAo9WnGWlRps+oG8BNOjxCOYzqrATfIuzfertROtjcIP9KlGZM7BPJWe72VoUeKMQcLYqGJVXTjcYE1/92ZH71FBIwhG9HOL3IZ3QliaDTLbAIZ892C5UD2af5eXvjeKB/ytHIxPOhhNJnJIQ8KppXw1UQW1c2BJSo1mxizp8nqf+ZcXOJ6nGmFWg1h9jyeSPwbeNy527+2SqlHDg69wmWhnNVkUvmj9Fx1S/5ZdGh6sj58z3MrJk3kUV8a9VSJhwR4UHBOGGULaoV1zPeB9jzTvWngGxHYKjGs1nKx8WhvzsFYinuj6Ap X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2020 21:44:15.3290 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: d92f88b2-613b-43fe-8266-08d7c2e0aea9 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB4093 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, MSGID_FROM_MTA_HEADER, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Mar 2020 21:44:21 -0000 David Malcolm writes: > On Mon, 2019-09-02 at 09:16 +0000, Andrea Corallo wrote: >> Hi all, >> yesterday I've found an interesting bug in libgccjit. >> Seems we have an hard limitation of 200 characters for literal >> strings. >> Attempting to create longer strings lead to ICE during pass_expand >> while performing a sanity check in get_constant_size. >> >> Tracking down the issue seems the code we have was inspired from >> c-family/c-common.c:c_common_nodes_and_builtins were >> array_domain_type >> is actually defined with a size of 200. >> The comment that follows that point sounded premonitory :) :) >> >> /* Make a type for arrays of characters. >> With luck nothing will ever really depend on the length of this >> array type. */ >> >> At least in the current implementation the type is set by >> fix_string_type were the actual string length is taken in account. >> >> I attach a patch updating the logic accordingly and a new testcase >> for that. >> >> make check-jit is passing clean. >> >> Best Regards >> Andrea > > Sorry about the long delay in reviewing this patch. > >> gcc/jit/ChangeLog >> 2019-??-?? Andrea Corallo >> >> * jit-playback.h >> (gcc::jit::recording::context m_recording_ctxt): Remove > ^^^^^^^^^ > "playback" here > >> m_char_array_type_node field. > > [...] > >> @@ -670,9 +669,12 @@ playback::rvalue * >> playback::context:: >> new_string_literal (const char *value) >> { >> - tree t_str = build_string (strlen (value), value); >> - gcc_assert (m_char_array_type_node); >> - TREE_TYPE (t_str) = m_char_array_type_node; >> + /* Compare with c-family/c-common.c: fix_string_type. */ >> + size_t len = strlen (value); >> + tree i_type = build_index_type (size_int (len)); >> + tree a_type = build_array_type (char_type_node, i_type); >> + tree t_str = build_string (len, value); >> + TREE_TYPE (t_str) = a_type; > > This code works with string lengths and string sizes which always > requires a little extra care. I'd like to see at least a comment > discussing this, as it's not immediately clear to me that we're > correctly handling the NUL terminator here. > > Consider the string "foo". > This has strlen == 3, and its size is 4 chars. > > build_string's comment says "Note that for a C string literal, LEN > should include the trailing NUL." > > However, build_string appears to allocate one byte more than LEN, and > write a NUL in that final position. > > fix_string_type has: > > int length = TREE_STRING_LENGTH (value); > > where tree.h has: > > /* In C terms, this is sizeof, not strlen. */ > #define TREE_STRING_LENGTH(NODE) (STRING_CST_CHECK (NODE)- >>string.length) > > and: > nchars = length / charsz; > > where for our purposes charsz == 1 and so nchars == length. > > fix_string_type has: > > i_type = build_index_type (size_int (nchars - 1)); > > So I *think* the patch is correct, but there ought to be a comment at > least. > > Or maybe use TREE_STRING_LENGTH (t_str) to more closely follow > fix_string_type? > Mmmm when I wrote the patch I looked into the build_string code missing the comment and that was my understanding too. But looking better now in the compiler I think the patch is *not* correct. The trouble is that if we call build_string (3, "foo") (as we would do now) we set TREE_STRING_LENGTH to 3 and this appears to be wrong. Actually build_string can be called using the strlen as argument (see for example the various definition of DEF_ATTR_STRING) but apparently this is not how is done for C strings in the frontends. Given that this is what the comment suggests and that modifying the patch with the +1 keeps it functional I think doing what the front-end does is the right thing. > Also, please add an assertion and comment to the testcase to assert > that the very long string is indeed longer than the previous limit of > 200. Ack > Thanks > Dave Thanks for reviewing Andrea