From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38428 invoked by alias); 6 Aug 2018 16:35:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 38419 invoked by uid 89); 6 Aug 2018 16:35:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=Investigation, consisted, tem, evaluating X-HELO: EUR03-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr50042.outbound.protection.outlook.com (HELO EUR03-VE1-obe.outbound.protection.outlook.com) (40.107.5.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 Aug 2018 16:35:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sYSbwnp7XwUqoMdBZRAszqpYIJgCDCCD1FtgMQkcUt0=; b=DsAhA8OxKBwL9vgGklzQm27fiFo0skRzus424/OQOyAhUDGQB2os5XrwzsMsbYY8u55Q8vk76KWwrVkUctOsR96fRY0n9cpHDvoeLHRdu8oHkIOopW+Fo2/sBFInLu4ZG61H9vvWZ4UqnVhy5yoqfLH4X6FA+D+4EaBsmhnUpsI= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Richard.Bunt@arm.com; Received: from [10.32.36.144] (217.140.106.40) by DB7PR08MB3193.eurprd08.prod.outlook.com (2603:10a6:5:1e::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1017.15; Mon, 6 Aug 2018 16:35:13 +0000 Subject: Re: [PATCH] Logical short circuiting with Fortran argument lists To: Simon Marchi Cc: gdb-patches@sourceware.org, nd@arm.com References: <20f669fe-9f31-fd39-9c3e-f2e1835576c6@arm.com> <002f6b4fc45a24efb3ce3582dcecdad0@polymtl.ca> From: Richard Bunt Message-ID: <9972887c-0dab-aef3-db1f-0e2323085dc7@arm.com> Date: Mon, 06 Aug 2018 16:35:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <002f6b4fc45a24efb3ce3582dcecdad0@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-Path: richard.bunt@arm.com Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00098.txt.bz2 On 08/03/2018 08:24 PM, Simon Marchi wrote: > On 2018-08-03 05:32, Richard Bunt wrote: >> Logical short circuiting with argument lists >> >> When evaluating Fortran expressions such as the following: >> >> print truth_table(1, 1) .OR. truth_table(2, 1) >> >> where truth_table(1, 1) evaluates to true, the debugger would report >> that it could not perform substring operations on this type. This patch >> addresses this issue. >> >> Investigation revealed that EVAL_SKIP was not being handled correctly >> for all types serviced by the OP_F77_UNDETERMINED_ARGLIST case in >> evaluate_subexp_standard. While skipping an undetermined argument >> list the type is resolved to be an integer (as this is what >> evaluate_subexp returns when skipping) and so it was not possible to >> delegate to the appropriate case (e.g. array, function call). >> >> The solution proposed here is to hoist the skip from the function call >> case to apply to all types of argument list. >> >> While this patch allows a wider range of expressions to be evaluated, >> it >> should be noted that this patch does not allow the skipping of arrays >> which use Fortran array slicing, due to the inability of the debugger >> to skip OP_RANGE. >> >> This patch has been tested with GCC 7.3 on x86_64, aarch64 and ppc64le. > > Hi Richard, > > Thanks for the patch, hat sounds reasonnable. Just one question: > >> diff --git a/gdb/eval.c b/gdb/eval.c >> index >> 9db6e7c69dad9e674f66991e2aee7dd8d66d80c7..2350c7faaf6221a5990cf5264fe7fe93b4f4be4c >> 100644 >> --- a/gdb/eval.c >> +++ b/gdb/eval.c >> @@ -1902,6 +1902,17 @@ evaluate_subexp_standard (struct type >> *expect_type, >> >> /* First determine the type code we are dealing with. */ >> arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside); >> + >> + /* Allow the short-circuiting of array accesses, function calls >> + and substring operations in logical expressions. */ >> + if (noside == EVAL_SKIP) >> + { >> + /* Skip all the array subscripts. */ >> + for (int i = 0; i < nargs; ++i) >> + evaluate_subexp (NULL_TYPE, exp, pos, noside); >> + return eval_skip_value (exp); >> + } >> + >> type = check_typedef (value_type (arg1)); >> code = TYPE_CODE (type); >> >> @@ -1952,8 +1963,6 @@ evaluate_subexp_standard (struct type >> *expect_type, >> for (; tem <= nargs; tem++) >> argvec[tem] = evaluate_subexp_with_coercion (exp, pos, noside); >> argvec[tem] = 0; /* signal end of arglist */ >> - if (noside == EVAL_SKIP) >> - return eval_skip_value (exp); > > Is this change needed? Your test passes even when leaving that part > as-is. Hi Simon, Many thanks for the review. To clarify, do you observe the test passing with no changes to eval.c or just when the additions are applied? If the former is the case, may I ask which compiler are you using? I have retested with 953473375500 and I see 13 tests fail without the additions. If it's the latter, the deletions are not strictly needed but my analysis determined that this code was unreachable after this patch. My analysis consisted of checking for regressions in the test suite (of which there were none) and examining all uses of noside from the new early termination to eval_call in TYPE_CODE_FUNC. There are no assignments and it's passed to all functions by value. I can separate this removal into a second patch if this is desirable. Rich > > Thanks, > > Simon >