From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 111336 invoked by alias); 1 Feb 2018 08:00:24 -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 84835 invoked by uid 89); 1 Feb 2018 08:00:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-7.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=firm, management, HContent-Transfer-Encoding:8bit X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Feb 2018 08:00:04 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 807AF56466; Thu, 1 Feb 2018 03:00:02 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id BgFls+d1RftO; Thu, 1 Feb 2018 03:00:02 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 87ADC56465; Thu, 1 Feb 2018 03:00:00 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id E69DF83307; Thu, 1 Feb 2018 11:59:55 +0400 (+04) Date: Thu, 01 Feb 2018 08:00:00 -0000 From: Joel Brobecker To: Wei-min Pan Cc: gdb-patches@sourceware.org Subject: Re: [PATCH PR gdb/20057] Internal error on trying to set {char[]}$pc="string" Message-ID: <20180201075955.mnqxzmw4ktuy3f5d@adacore.com> References: <1516844738-79996-1-git-send-email-weimin.pan@oracle.com> <20180125041431.tghhxefsgxnxh3l3@adacore.com> <1dfc87b0-353d-3388-a427-fee247dc79a5@oracle.com> <20180131074526.rqbsjxyxp3p26js5@adacore.com> <1d28e9c6-6377-0c46-6bce-1dc25a7fa2d5@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1d28e9c6-6377-0c46-6bce-1dc25a7fa2d5@oracle.com> User-Agent: NeoMutt/20170113 (1.7.2) X-SW-Source: 2018-02/txt/msg00001.txt.bz2 > > Unfortunately, I only have vague answers for you. I know it's not > > as satisfactory as a firm one, but I haven't had time to investigate > > further. > > > > My feeling is that it's (intuitively) a bad idea to start mixing > > and matching the ownership type for a give type chain. It just > > muddies the waters, and makes memory management more complex. > > Given there are functions such as arch_integer_type(), > arch_character_type(), > and arch_float_type() that can be used to add types to an arch, it doesn't > seem terribly wrong to add a type which is not associated with any objfile > to gdbarch? Also a type can actually exist in both an arch and an objfile. I am not sure we understand each other. For me, what seems wrong is the fact that we have an array type where part of the type is objfile-owned, and part of it arch-owned. Creating arch-owned type is fine, as long as the entire type is arch-owned. > > Parallel to that, there is another obstacle if you want to enhance > > copy_type to handle arch-owned types, as the current implementation > > explicitly assumes that the type is objfile-owned, and therefore > > references its objfile's obstack: > > > > if (TYPE_DYN_PROP_LIST (type) != NULL) > > TYPE_DYN_PROP_LIST (new_type) > > = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack, > > TYPE_DYN_PROP_LIST (type)); > > Good point. The following statement > >   if (TYPE_DYN_PROP_LIST (type) != NULL) > > needs to be changed to: > >   if (TYPE_DYN_PROP_LIST (type) != NULL && TYPE_OBJFILE_OWNED(type)) That would be wrong, because the resulting type would be missing that dynamic property list, which means the resulting type would be a complete copy of the original type. It's not so simple! > Your fix in lookup_array_range_type() takes care the case where > "element_type" was owned by an objfile but still creates an arch-owned > index type if it was not. That is correct, and it is not a problem as long as the entire type is consistent. > Here is the test case that comes with the PR: > > % cat x.c > char p[] = "hello"; > > int main() > { >   return ((int)(p[0])); > } > > Please note that the test case declares base type "char" which has an > associated objfile and is picked up by lookup_symbol_aux() when > command "set {char[]}$pc="hi" is parsed and eventually is passed as > the element type argument to lookup_array_range_type(). Using any > other type, such as "unsigned char", in that gdb command results in > the element type that is picked up from gdbarch and has no associated > objfile. That is exactly the problem. At the point where it decides to use an arch-owned type, it should check the type it is for, and whether it is arch or objfile owned, and then create the type from there. If my intuition is right, my patch should be a good example of what needs to be done. -- Joel