From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id H4XJID8uPmDAPQAAWB0awg (envelope-from ) for ; Tue, 02 Mar 2021 07:23:27 -0500 Received: by simark.ca (Postfix, from userid 112) id 7AC651EF78; Tue, 2 Mar 2021 07:23:27 -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 4A3B21E793 for ; Tue, 2 Mar 2021 07:23:26 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E6679388C01D; Tue, 2 Mar 2021 12:23:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E6679388C01D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1614687805; bh=Y2AzT8ejdLWrBtbdR1pIkRAtSSBxfVA1EpOvoYpB160=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=idgumAmxmATAi9P3ZjLy0JqBGwrxIVgOJ5zxG+QvZJ09e519mgrEDNn8EYUACv5t4 BZ2DXM/Yr6RgQlhHxqa63nRvsXnamlJ9ze7VKr9REW6ZSRkToTshr0I6iSEOtOstmy U5octD1qgxRgHAZ8O6mvp28ZId1QQ/RUxVYp6duA= Received: from beryx.lancelotsix.com (beryx.lancelotsix.com [164.132.98.193]) by sourceware.org (Postfix) with ESMTPS id 8A8473858034 for ; Tue, 2 Mar 2021 12:23:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8A8473858034 Received: from Plymouth (unknown [IPv6:2a02:390:8443:0:a5f1:812d:cfee:e3ba]) by beryx.lancelotsix.com (Postfix) with ESMTPSA id 1CABB2E03B; Tue, 2 Mar 2021 13:23:21 +0100 (CET) Date: Tue, 2 Mar 2021 12:23:19 +0000 To: Weimin Pan Subject: Re: [PATCH,V2 2/5] CTF: set up debug info for function arguments Message-ID: References: <1614650018-9135-1-git-send-email-weimin.pan@oracle.com> <1614650018-9135-2-git-send-email-weimin.pan@oracle.com> <1614650018-9135-3-git-send-email-weimin.pan@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1614650018-9135-3-git-send-email-weimin.pan@oracle.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.11 (beryx.lancelotsix.com [0.0.0.0]); Tue, 02 Mar 2021 13:23:21 +0100 (CET) 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: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi, I just have style related comments bellow. Le Mon, Mar 01, 2021 at 08:53:35PM -0500, Weimin Pan via Gdb-patches a écrit : > Added this support in read_func_kind_type after gcc started generating > CTF for function arguments. > > Expanded gdb.base/ctf-ptype.exp to test function arguments. Also fixed > some typos. > > --- > gdb/ChangeLog | 4 ++++ > gdb/ctfread.c | 29 ++++++++++++++++++++++++++++- > gdb/testsuite/ChangeLog | 4 ++++ > gdb/testsuite/gdb.base/ctf-ptype.exp | 19 ++++++++++++++++--- > 4 files changed, 52 insertions(+), 4 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 88278d1..0a839ba 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,5 +1,9 @@ > 2021-02-26 Weimin Pan > > + * ctfread.c (read_func_kind_type): Set up function arguments. > + > +2021-02-26 Weimin Pan > + > * ctfread.c (new_symbol): Set function address. > (read_func_kind_type): Remove incorrect type name setting. > > diff --git a/gdb/ctfread.c b/gdb/ctfread.c > index 5a68d9c..75b098c 100644 > --- a/gdb/ctfread.c > +++ b/gdb/ctfread.c > @@ -652,8 +652,9 @@ struct ctf_tid_and_type > { > struct objfile *of = ccp->of; > ctf_dict_t *fp = ccp->fp; > - struct type *type, *rettype; > + struct type *type, *rettype, *atype; > ctf_funcinfo_t cfi; > + uint32_t argc; > > type = alloc_type (of); > > @@ -663,6 +664,32 @@ struct ctf_tid_and_type > TYPE_TARGET_TYPE (type) = rettype; > set_type_align (type, ctf_type_align (fp, tid)); > > + /* Set up function's arguments. */ > + argc = cfi.ctc_argc; > + type->set_num_fields (argc); > + if (cfi.ctc_flags & CTF_FUNC_VARARG) > + type->set_has_varargs (true); > + > + if (argc != 0) > + { > + std::vector argv (argc); > + if (ctf_func_type_args (fp, tid, argc, argv.data ()) == CTF_ERR) > + return NULL; Could be nullptr instead of NULL (even if I am not sure the standard enforces that). > + > + type->set_fields > + ((struct field *) TYPE_ZALLOC (type, argc * sizeof (struct field))); > + struct type *void_type = objfile_type (of)->builtin_void; > + /* If failed to find the argument type, fill it with void_type. */ > + for (int iparam = 0; iparam < argc; iparam++) > + { > + atype = get_tid_type (of, argv[iparam]); > + if (atype) An explicit comparison against nullptr should be done here. See https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_NULL_And_Zero Thanks, Lancelot. > + type->field (iparam).set_type (atype); > + else > + type->field (iparam).set_type (void_type); > + } > + } > + > return set_tid_type (of, tid, type); > } > > diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog > index 3d71354..9445362 100644 > --- a/gdb/testsuite/ChangeLog > +++ b/gdb/testsuite/ChangeLog > @@ -1,5 +1,9 @@ > 2021-02-26 Weimin Pan > > + * gdb.base/ctf-ptype.exp: Add function tests and fix typos. > + > +2021-02-26 Weimin Pan > + > * gdb.base/ctf-funcreturn.exp: New file. > > 2021-01-29 Tom de Vries > diff --git a/gdb/testsuite/gdb.base/ctf-ptype.exp b/gdb/testsuite/gdb.base/ctf-ptype.exp > index ffe40f1..056f712 100644 > --- a/gdb/testsuite/gdb.base/ctf-ptype.exp > +++ b/gdb/testsuite/gdb.base/ctf-ptype.exp > @@ -63,10 +63,10 @@ gdb_test "ptype struct t_struct" "type = struct t_struct \{.*\[\r\n\] (unsign > > # Test the equivalence between '.' and '->' for struct member references. > > -if [gdb_test "ptype v_t_struct_p.v_float_member" "type = float"]<0 then { > +if [gdb_test "ptype v_struct1.v_float_member" "type = float"]<0 then { > return -1 > } > -if [gdb_test "ptype v_t_struct_p->v_float_member" "type = float"]<0 then { > +if [gdb_test "ptype v_struct1->v_float_member" "type = float"]<0 then { > return -1 > } > if [gdb_test "ptype v_t_struct_p.v_float_member" "type = float"]<0 then { > @@ -211,7 +211,7 @@ gdb_test "ptype the_highest" \ > > gdb_test "ptype the_highest.anonymous_level_1" \ > "type = struct \{.*\[\r\n\] *int b;.*\[\r\n\] *struct \{.*\[\r\n\] *int c;.*\[\r\n\] *\} anonymous_level_2;.*\[\r\n\]}.*" \ > - "ptype the_highest" > + "ptype the_highest.anonymous_level_1" > > # Print the type of the identifier ID, and check the response: > # - Expect to see PROTOTYPED as the type. PROTOTYPED is not a regular > @@ -255,8 +255,21 @@ proc ptype_maybe_prototyped { id prototyped plain { overprototyped "NO-MATCH" } > } > } > > +ptype_maybe_prototyped "func_type" "int (*)(int (*)(int, float), float)" \ > + "int (*)()" > ptype_maybe_prototyped "old_fptr" "double (*)()" "double (*)()" \ > "double (*)(void)" > +ptype_maybe_prototyped "new_fptr" "double (*)()" "double (*)()" > +ptype_maybe_prototyped "fptr" "int (*)(int, float)" "int (*)()" > +ptype_maybe_prototyped "fptr2" "int *(*)(int (*)(int, float), float)" \ > + "int *(*)()" > +ptype_maybe_prototyped "xptr" "int (*)(int (*)(), int (*)(), int)" \ > + "int (*)()" \ > + "int (*)(int (*)(void), int (*)(void), int)" > +ptype_maybe_prototyped "ffptr" "int (*(*)(char))(short int)" \ > + "int (*(*)())()" > +ptype_maybe_prototyped "fffptr" "int (*(*(*)(char))(short int))(long int)" \ > + "int (*(*(*)())())()" > > # Test printing type of string constants and array constants, but > # requires a running process. These call malloc, and can take a long > -- > 1.8.3.1 > -- Lancelot SIX