From patchwork Thu Mar 9 01:58:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Seitz X-Patchwork-Id: 19497 Received: (qmail 59352 invoked by alias); 9 Mar 2017 01:58:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 59320 invoked by uid 89); 9 Mar 2017 01:58:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No 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, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 09 Mar 2017 01:58:20 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C01856AAE6 for ; Thu, 9 Mar 2017 01:58:19 +0000 (UTC) Received: from valrhona.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v291wG1A024876 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 8 Mar 2017 20:58:18 -0500 Subject: Re: [PATCH] c++/8218: Destructors w/arguments. To: gdb-patches@sourceware.org References: <1489010611-28451-1-git-send-email-keiths@redhat.com> <2ca278e9-a52a-c082-77f5-5ed60e39091e@redhat.com> From: Keith Seitz Message-ID: <58C0B6B6.9040008@redhat.com> Date: Wed, 8 Mar 2017 17:58:14 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <2ca278e9-a52a-c082-77f5-5ed60e39091e@redhat.com> X-IsSubscribed: yes On 03/08/2017 02:53 PM, Pedro Alves wrote: >> gdb/testsuite/ChangeLog >> >> PR c++/8218 >> * c-typeprint.c (cp_type_print_method_args): Start printing arguments >> at index 0, ignoring STATCIP. >> Skip artificial arguments. > > Wrong entry for testsuite. Cut-n-paste-o. Fixed/updated. >> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c >> index 75f6d61..e504618 100644 >> --- a/gdb/c-typeprint.c >> +++ b/gdb/c-typeprint.c >> @@ -226,13 +226,18 @@ cp_type_print_method_args (struct type *mtype, const char *prefix, >> language_cplus, DMGL_ANSI); >> fputs_filtered ("(", stream); >> >> - /* Skip the class variable. */ >> - i = staticp ? 0 : 1; >> + i = 0; >> if (nargs > i) >> { >> while (i < nargs) >> { >> - c_print_type (args[i++].type, "", stream, 0, 0, flags); >> + struct field arg = args[i++]; > > The manipulation of 'i' looks a bit obscure. This is crying > for a "for", IMO: Yeah, that could have been done a little better, but given your concerns below, I've reverted this bit. Better safe than sorry. > Can we trust that FIELD_ARTIFICIAL is always set on the implicit this > argument on all debug formats? I.e., does it work with stabs? No, only DWARF, I think. I've tested gdb.cp/*.exp for -gdwarf-2 and -gstabs+ for both 64-bit and -m32. In all cases, with or without my v1 proposed patch, logs show no differences whatsoever, except in the -gdwarf-2 cases where the modified templates.exp tests now pass. The STABS cases never pass the two (templates.exp) tests. Nonetheless, I've simplified my changes, maintaining the "if non-static, always skip the first argument". I simply also skip any subsequent artificial arguments in the loop. [I've also added a comment about why we always skip the first argument for non-static functions.] > Can you comment on the treatment c_type_print_args gives to > artificial arguments that Tom pointed out in the PR? As Tom mentions, c_type_print_args will output the types of function arguments /unless/ it is being called from the dwarf reader (to construct physnames). This function is called when someone does "ptype A::~A". It is /not/ used when one types "ptype A". The dtor in that instance is handled by cp_type_print_method_args. Detailed explanation of why we don't see the artificial "int" parameter in both cases (before my patch): Because of how the compiler is outputting dtor debug info, we see multiple DIEs describing the destructor. When one types "ptype A", this uses one of the DIEs. This particular one mentions the two artificial parameters. That's why we see (before this patch) the "int" parameter. When one types "ptype A::~A", this uses a different DIE, which has valid PC bounds, but only mentions the first formal parameter. This is why when we see only the "const A *" in the description of the parameters. I've no explanation as to why we don't hide these implementation details other than to offer a truly pedantic view of the code. As best I can describe it, then, "ptype A::~A" (aka c_type_print_args) attempts to be as tools-developer-oriented as possible while "ptype CLASS" attempts to be more user-centric, hiding internal details like artificial constructs. Did any of that actually answer your question? > Also that function's intro comment talks about C++ "this". > Is that stale? You mean c_type_print_args? I believe that comment is accurate. Keith \\((void|)\\);${ws}\\}${ws}$gdb_prompt $" { xfail "ptype T5 -- new with unsigned int" } - -re "type = class T5 \\{.*public:.*static int X;.*int x;.*int val;.*T5 \\(int\\);.*T5 \\(const class T5 &\\);.*void ~T5 \\(int\\);.*static void \\* new \\(unsigned long\\);.*static void delete \\(void ?\\*\\);.*int value \\((void|)\\);.*\\}\r\n$gdb_prompt $" { + -re "type = class T5 \\{.*public:.*static int X;.*int x;.*int val;.*T5 \\(int\\);.*T5 \\(const class T5 &\\);.*void ~T5 \\(\\);.*static void \\* new \\(unsigned long\\);.*static void delete \\(void ?\\*\\);.*int value \\((void|)\\);.*\\}\r\n$gdb_prompt $" { xfail "ptype T5 -- new with unsigned long" } -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;((${ws}T5 & operator=\\(T5 const ?&\\);)|(${ws}T5\\(int\\);)|(${ws}T5\\((T5 const|const T5) ?&\\);)|(${ws}~T5\\((void|)\\);)|(${ws}static void \\* operator new\\(unsigned( int| long)?\\);)|(${ws}static void operator delete\\(void ?\\*\\);)|(${ws}int value\\((void|)\\);))*${ws}\}\r\n$gdb_prompt $" { xfail "ptype T5 (obsolescent gcc or gdb)" } - -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}void T5\\(int\\);${ws}void T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(int\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { + -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}void T5\\(int\\);${ws}void T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { # This also triggers gdb/1113... kfail "gdb/1111" "ptype T5" # Add here a PASS case when PR gdb/1111 gets fixed. @@ -68,19 +68,22 @@ proc test_ptype_of_templates {} { # The destructor has an argument type. kfail "gdb/8218" "ptype T5" } + -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { + pass "ptype T5" + } } gdb_test_multiple "ptype/r t5i" "ptype t5i" { -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;\r\n${ws}T5\\(int\\);${ws}T5\\(T5 const ?&\\);${ws}~T5\\((void|)\\);${ws}static void \\* operator new\\(unsigned( int| long)?\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\\}\r\n$gdb_prompt $" { xfail "ptype T5 -- with several fixes from 4.17 -- without size_t" } - -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;\r\n${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(int\\);${ws}static void \\* new \\(unsigned int\\);${ws}static void delete \\(void ?\\*\\);${ws}int value \\((void|)\\);${ws}\\}\r\n$gdb_prompt $" { + -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;\r\n${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(\\);${ws}static void \\* new \\(unsigned int\\);${ws}static void delete \\(void ?\\*\\);${ws}int value \\((void|)\\);${ws}\\}\r\n$gdb_prompt $" { xfail "ptype t5i -- new with unsigned int -- without size_t" } - -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;\r\n${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(int\\);${ws}static void \\* new \\(unsigned long\\);${ws}static void delete \\(void ?\\*\\);${ws}int value \\((void|)\\);${ws}\\}\r\n$gdb_prompt $" { + -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;\r\n${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(\\);${ws}static void \\* new \\(unsigned long\\);${ws}static void delete \\(void ?\\*\\);${ws}int value \\((void|)\\);${ws}\\}\r\n$gdb_prompt $" { xfail "ptype t5i -- new with unsigned long -- without size_t" } - -re "type = class T5 \{.*public:.*static int X;.*int x;.*int val;.*.*T5 \\(int\\);.*.*void ~T5 \\(int\\).*.*.*int value \\((void|)\\);.*\}.*$gdb_prompt $" { + -re "type = class T5 \{.*public:.*static int X;.*int x;.*int val;.*.*T5 \\(int\\);.*.*void ~T5 \\(\\).*.*.*int value \\((void|)\\);.*\}.*$gdb_prompt $" { xfail "ptype t5i -- without size_t" } -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5 & operator=\\(T5 const ?&\\);${ws}T5\\(int\\);${ws}T5\\((T5 const|const T5) ?&\\);${ws}~T5\\((void|)\\);${ws}static void \\* operator new\\(unsigned( int| long)?\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { @@ -92,9 +95,9 @@ proc test_ptype_of_templates {} { -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;((${ws}T5 & operator=\\(T5 const ?&\\);)|(${ws}T5\\(int\\);)|(${ws}T5\\(T5 const ?&\\);)|(${ws}~T5\\((void|)\\);)|(${ws}static void \\* operator new\\(unsigned( int| long)?\\);)|(${ws}static void operator delete\\(void ?\\*\\);)|(${ws}int value\\((void|)\\);))*${ws}\}\r\n$gdb_prompt $" { xfail "ptype t5i (obsolescent gcc or gdb) -- without size_t" } - -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}void T5\\(int\\);${ws}void T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(int\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { + -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}void T5\\(int\\);${ws}void T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { # This also triggers gdb/1113... - kfail "gdb/1111" "ptype T5" + kfail "gdb/1111" "ptype t5i" # Add here a PASS case when PR gdb/1111 gets fixed. # These are really: # http://sourceware.org/bugzilla/show_bug.cgi?id=8216 @@ -103,7 +106,10 @@ proc test_ptype_of_templates {} { -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(int\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { # http://sourceware.org/bugzilla/show_bug.cgi?id=8218 # The destructor has an argument type. - kfail "gdb/8218" "ptype T5" + kfail "gdb/8218" "ptype t5i" + } + -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5 const|const T5) ?&\\);${ws}~T5\\(\\);${ws}static void \\* operator new\\((size_t|unsigned( int| long|))\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}\}\r\n$gdb_prompt $" { + pass "ptype t5i" } } } diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3ac5170..ca7e5e2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2017-03-08 Keith Seitz + + PR c++/8218 + * c-typeprint.c (cp_type_print_method_args): Skip artificial arguments. + 2017-02-21 Jan Kratochvil * dwarf2read.c (dwarf2_record_block_ranges): Add forgotten BASEADDR. diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c index 75f6d61..b97216e 100644 --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -226,13 +226,21 @@ cp_type_print_method_args (struct type *mtype, const char *prefix, language_cplus, DMGL_ANSI); fputs_filtered ("(", stream); - /* Skip the class variable. */ + /* Skip the class variable. We keep this here to accommodate older + compilers and debug formats which may not support artificial + parameters. */ i = staticp ? 0 : 1; if (nargs > i) { while (i < nargs) { - c_print_type (args[i++].type, "", stream, 0, 0, flags); + struct field arg = args[i++]; + + /* Skip any artificial arguments. */ + if (FIELD_ARTIFICIAL (arg)) + continue; + + c_print_type (arg.type, "", stream, 0, 0, flags); if (i == nargs && varargs) fprintf_filtered (stream, ", ..."); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 420bfc4..b26d800 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2017-03-08 Keith Seitz + + PR c++/8128 + * gdb.cp/templates.exp (test_ptype_of_templates): Remove argument + type from destructor regexps. + Add a branch which actually passes the test. + Adjust "ptype t5i" test names. + 2017-02-21 Edjunior Barbosa Machado * gdb.arch/ppc64-isa207-atomic-inst.exp: New testcase based on diff --git a/gdb/testsuite/gdb.cp/templates.exp b/gdb/testsuite/gdb.cp/templates.exp index 6f9131f..85f86ee 100644 --- a/gdb/testsuite/gdb.cp/templates.exp +++ b/gdb/testsuite/gdb.cp/templates.exp @@ -46,16 +46,16 @@ proc test_ptype_of_templates {} { -re "type = class T5 \{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}T5\\(int\\);${ws}T5\\((T5 const|const T5) ?&\\);${ws}~T5\\((void|)\\);${ws}static void \\* operator new\\(unsigned( int| long)?\\);${ws}static void operator delete\\(void ?\\*\\);${ws}int value\\((void|)\\);${ws}T5 & operator=\\(T5 const ?&\\);${ws}\}\r\n$gdb_prompt $" { xfail "ptype T5 -- new without size_t" } - -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(int\\);${ws}static void \\* new \\(unsigned int\\);${ws}static void delete \\(void ?\\*\\);${ws}int value \\((void|)\\);${ws}\\}${ws}$gdb_prompt $" { + -re "type = class T5 \\{${ws}public:${ws}static int X;${ws}int x;${ws}int val;${ws}${ws}T5 \\(int\\);${ws}T5 \\(const class T5 &\\);${ws}void ~T5 \\(()\\);${ws}static void \\* new \\(unsigned int\\);${ws}static void delete \\(void ?\\*\\);${ws}int value