From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id K9NYB/tm/F9hGwAAWB0awg (envelope-from ) for ; Mon, 11 Jan 2021 09:55:55 -0500 Received: by simark.ca (Postfix, from userid 112) id 106981EEEF; Mon, 11 Jan 2021 09:55:55 -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 218191E4F4 for ; Mon, 11 Jan 2021 09:55:54 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3359B3858001; Mon, 11 Jan 2021 14:55:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3359B3858001 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610376953; bh=C3Nxk3Ae7/xwpdSmL79AD2tE+1CscL9gWqo+zps4hK4=; 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=pvycNT34foMl6mn2At+nuc4k0Yx0Bd7I81rYMGcQWEpYPQH5u+tUupVXz+Da1UpCd ss6Z8cXr+BTMIuAYAnueTIMfzl2TE47raHGV/4/gE6o3YNfer6D3qHXZvpm49ekw22 p74dVVxTjVh09Dp07UcDfAw8bFDTqUz6psyIYUZA= Received: from mail-qt1-x829.google.com (mail-qt1-x829.google.com [IPv6:2607:f8b0:4864:20::829]) by sourceware.org (Postfix) with ESMTPS id 741B33858001 for ; Mon, 11 Jan 2021 14:55:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 741B33858001 Received: by mail-qt1-x829.google.com with SMTP id c14so11372644qtn.0 for ; Mon, 11 Jan 2021 06:55:50 -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:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=C3Nxk3Ae7/xwpdSmL79AD2tE+1CscL9gWqo+zps4hK4=; b=jBu8JziXOAklSVaA/7aq/mQK0IlcdYPmTOpEqgNLaCmsfqGguRX8qs9+bAU/mF4wso FWMUBhDfLeSlQb4rNv9xDKLS1Rvh2QRl/wti/fk2HCpB3+Pckf8Nw5pNTVtsaP53Y9pE 2pFN0H0fCobK/W10UkmU7ocPLNJQ+42rvHouVPUpnVbSIGpvtVS8JFep/VMgk3g11gU7 k2z8YG76wTKVk2AoDbzidWLa4V315kUoNCkyfBMmVTP+IMDIYiHOu/h01R3SMSGCJFd7 dUl6GlLCFwSzW9ENlktFrs0Z5ZY3JX4KuuEst7+D4CWWQ+CAviG0I9BfTnxta7eoHwF9 OcZQ== X-Gm-Message-State: AOAM532FatLWotRALmCa0IQaQFXApiFSouOomLmlGROuXnyk2wyQJ4zl d6yF5FGGJMFEIf6XFavoA7fkuzyjKQAV8A== X-Google-Smtp-Source: ABdhPJzeuWWXAzOKtXQrTyla1wuc1CLGejIrxA4/ZpqXYFuzUDtYXHQmCU2jH1exULjooErEdb789A== X-Received: by 2002:ac8:5c41:: with SMTP id j1mr1847590qtj.306.1610376949846; Mon, 11 Jan 2021 06:55:49 -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 z10sm8332482qtm.54.2021.01.11.06.55.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 Jan 2021 06:55:49 -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> <918220e8-b255-d575-3e45-48cb53c20f9d@linaro.org> Message-ID: Date: Mon, 11 Jan 2021 11:55:46 -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: <918220e8-b255-d575-3e45-48cb53c20f9d@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" On 1/11/21 11:30 AM, Luis Machado wrote: > 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. I think this has been addressed already in a later change. I just noticed some libiberty change broke the HEAD build and I was sitting at a GDB from Jan 4th. This is a non-issue then.