From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id I0fSOgZh/F/EGgAAWB0awg (envelope-from ) for ; Mon, 11 Jan 2021 09:30:30 -0500 Received: by simark.ca (Postfix, from userid 112) id E43A81EEEF; Mon, 11 Jan 2021 09:30:30 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C4A4D1E4F4 for ; Mon, 11 Jan 2021 09:30:29 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0342F385800A; Mon, 11 Jan 2021 14:30:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 0342F385800A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610375429; bh=OBsYvn1+JMMWBNFOgUFOdKwhhhTI4jEPk4Dhvqk+bR4=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=LhRcbAvDMKyvmkaEVpl0ITeP9nh1Uqh/Vrf+VNGpryp+uMP44iKP0QNAlEHH6m8rn vQ1IHvlkrjG0FJbdEyaGCyvoWzC/lcW0ZHtsgtnseq6kiRLDVGYOtfN+5ZyRXABuXV oJdWbahz9ozciqyPVjii1riA6O1Mug48X++ZznXQ= Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id 03400385800A for ; Mon, 11 Jan 2021 14:30:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 03400385800A Received: by mail-qk1-x72e.google.com with SMTP id h4so14691031qkk.4 for ; Mon, 11 Jan 2021 06:30:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=OBsYvn1+JMMWBNFOgUFOdKwhhhTI4jEPk4Dhvqk+bR4=; b=HgB6cO9b+ZngBTRKBaDEW+s//VWowx6KSLMkaX0lXzexdYF07vfY7RkY5XEfzRekOf MInZmg4VCSzr4PcU2SzJQzba3Dqj+rjVZV6Gy1K6cFVW8HydaXAIK1pFWqkZUVg85VCg LjkfAXrRHExgHlXUkOBr28xGYNMbieECG1xN13xdVl78cF/NvEFEHA5q+c/seDoZ5vEN O6u65DGfk+jobezoGhiDadRq9ixiwBIAHGHHeDKkocKG+bFxl6ztV8SavxDVb5cgl2Gt 1vsRzv3quCNW6lGozBGB1Aqr1OY/z7gRxoBFg/8jbXKy74NAqApvpNuZU6177BYeCDBW HVTQ== X-Gm-Message-State: AOAM5332dRp2AFMmvrD7ZrrRek8NGm3/RiQzEO0dji/i9v2E/37W8BiF zPOMFcdGoiQ5Ak0tqt092ljjqpqnxHfcxQ== X-Google-Smtp-Source: ABdhPJzO0tyQvOpVnikdhBVn3e48+gTL9AUDleXwf7FgadLOeCh/5q+lUfVpHlapPV5HUPbvHcrqDA== X-Received: by 2002:a37:9d84:: with SMTP id g126mr16711852qke.262.1610375425275; Mon, 11 Jan 2021 06:30:25 -0800 (PST) Received: from ?IPv6:2804:7f0:8284:874d:20e9:a3d4:1db5:c30a? ([2804:7f0:8284:874d:20e9:a3d4:1db5:c30a]) by smtp.gmail.com with ESMTPSA id x47sm8042301qtb.86.2021.01.11.06.30.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 Jan 2021 06:30:23 -0800 (PST) Subject: Re: [PATCH] gdb: user variables with components of dynamic type To: Andrew Burgess , Joel Brobecker References: <20201022153238.1947197-1-andrew.burgess@embecosm.com> <20201115142458.GA509764@adacore.com> <20210108115605.GY2945@embecosm.com> Message-ID: <918220e8-b255-d575-3e45-48cb53c20f9d@linaro.org> Date: Mon, 11 Jan 2021 11:30:20 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20210108115605.GY2945@embecosm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Luis Machado via Gdb-patches Reply-To: Luis Machado Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi, On 1/8/21 8:56 AM, Andrew Burgess wrote: > * Joel Brobecker [2020-11-15 18:24:58 +0400]: > >> Hi Andrew, >> >>> - /* If type has a dynamic resolved location property >>> - update it's value address. */ >>> + /* If either the WHOLE value, or the COMPONENT value has a dynamic >>> + resolved location property then update the address of the COMPONENT. >>> + >>> + If the COMPONENT itself has a dynamic location, and was an >>> + lval_internalvar_component, then we change this to lval_memory. >>> + Usually a component of an internalvar is created non-lazy, and has its >>> + content immediately copied from the parent internalvar. However, >>> + for components with a dynamic location, the content of the component >>> + is not contained within the parent, but is instead accessed >>> + indirectly. Further, the component will be created as a lazy value. >>> + >>> + By changing the type of the component to lval_memory we ensure that >>> + value_fetch_lazy can successfully load the component. */ >>> type = value_type (whole); >>> if (NULL != TYPE_DATA_LOCATION (type) >>> && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST) >>> set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); >>> + >>> + type = value_type (component); >>> + if (NULL != TYPE_DATA_LOCATION (type) >>> + && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST) >>> + { >>> + if (VALUE_LVAL (component) == lval_internalvar_component) >>> + { >>> + gdb_assert (value_lazy (component)); >>> + VALUE_LVAL (component) = lval_memory; >>> + } >>> + else >>> + gdb_assert (VALUE_LVAL (component) == lval_memory); >>> + set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); >>> + } >> >> I have a suggestion, but I am not sure it might be right for >> everyone, as perhaps other people's brains might be thinking >> differently. >> >> In your patch you architected it with one large comment at beginning >> followed by a number of conditinal branches, with the comment explaining >> the various scenarios that we're about to handle. If your brain works >> like mine, I would find the following approach to make it easier for me >> to understand the code: >> >> type = value_type (component); >> if (NULL != TYPE_DATA_LOCATION (type) >> && TYPE_DATA_LOCATION_KIND (type) == PROP_CONST) >> { >> /* COMPONENT's type has a dynamic location, so we need >> to update our component's address to reflect the actual >> location after resolution. */ >> if (VALUE_LVAL (component) == lval_internalvar_component) >> { >> /* This happens when [...]. >> We have to change the lval to lval_memory because ... */ >> gdb_assert (value_lazy (component)); >> VALUE_LVAL (component) = lval_memory; >> } >> >> /* At this point, we assume that COMPONENT is now an lval_memory, >> and we can now set it address. */ >> gdb_assert (VALUE_LVAL (component) == lval_memory); >> set_value_address (component, TYPE_DATA_LOCATION_ADDR (type)); >> } >> >> As I mentioned, maybe you don't think the read code the same way >> as I do, and so it would be absolutely fine with me if you don't >> agree with the suggestion ;-). > > After rereading the our other discussion of this patch I believe the > conclusion was that you don't object to this patch. > > As nobody else has commented I went ahead and pushed the version > below. > > The only changes are: > > - Split the single big comment up as you suggested, and > - Tweaked some wording in the commit log. > > Thanks, > Andrew This seems to be causing some internal errors on AArch64-Linux Ubuntu 18.04. FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a (GDB internal error) FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_one field (GDB internal error) FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print array_two field (GDB internal error) FAIL: gdb.fortran/intvar-dynamic-types.exp: print $a: print full contents (GDB internal error) FAIL: gdb.fortran/intvar-dynamic-types.exp: print $b after a change FAIL: gdb.fortran/intvar-dynamic-types.exp: print $c after a change FAIL: gdb.fortran/intvar-dynamic-types.exp: print some_var FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_one(2,2) = 3 FAIL: gdb.fortran/intvar-dynamic-types.exp: set $a%array_two(3,1) = 4 FAIL: gdb.fortran/intvar-dynamic-types.exp: set $b(2,2) = 3 FAIL: gdb.fortran/intvar-dynamic-types.exp: set $c(3,1) = 4 The internal error is... print $a $1 = ( array_one = ../../../repos/binutils-gdb/gdb/value.c:3983: internal-error: Unexpected lazy value type. Any ideas? I can provide the full log as well, if you think that is useful.