From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 130917 invoked by alias); 14 Sep 2018 21:49:14 -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 130775 invoked by uid 89); 14 Sep 2018 21:48:58 -0000 Authentication-Results: sourceware.org; auth=none 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,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=H*Ad:D*oracle.com X-HELO: userp2130.oracle.com Received: from userp2130.oracle.com (HELO userp2130.oracle.com) (156.151.31.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Sep 2018 21:48:56 +0000 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w8ELmqrx136902; Fri, 14 Sep 2018 21:48:52 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2018-07-02; bh=pRlNhHiRnloaKH55luIDc7J029fln7gfnWPZPPJKe48=; b=qsW3Q7alt1D1XL8IFR+sgDDphSpjXsxzqvfI+x1Sqy5TWOXUfh9vuCEK8VstoT1RMyYS Fkv3QUz0M54AOch26CdkhvPfiwsb+Gux/Nvyap7sF0vh0AAdJh3AFm+/ElLi2wgdYt95 oqNod8i7xxSEEP3q4CqKTJqm8XIgWqJi8mT8miNYu/KUpCs/lvUQ/73emGVk5cs7N0bb 1pBK+ptD2YqkfA+37oiiVpyW2RJgTm7TmKISVwvZ7oBlM4H5dKRZTxFb4dm722i54RlG n/D832uaOvA6FjIK1OMrlb45ccO9gHZwvXQ50CkNSueDZPsXqkJSFyyTurB1s8TYK/00 jQ== Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp2130.oracle.com with ESMTP id 2mc5uu1a3f-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 14 Sep 2018 21:48:51 +0000 Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w8ELmnll021510 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 14 Sep 2018 21:48:49 GMT Received: from abhmp0013.oracle.com (abhmp0013.oracle.com [141.146.116.19]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w8ELmmW4008125; Fri, 14 Sep 2018 21:48:49 GMT Received: from [10.132.97.56] (/10.132.97.56) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 14 Sep 2018 14:48:48 -0700 Subject: Re: [PATCH v3 PR gdb/16841] virtual inheritance via typedef cannot find base To: Simon Marchi , gdb-patches@sourceware.org Cc: Tom Tromey References: <1536800471-78975-1-git-send-email-weimin.pan@oracle.com> <136d54d3-7ee2-6640-5858-10831f53b8b1@ericsson.com> From: Weimin Pan Message-ID: <2f208dca-d74b-3d8c-afcf-f6da828bc760@oracle.com> Date: Fri, 14 Sep 2018 21:49:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <136d54d3-7ee2-6640-5858-10831f53b8b1@ericsson.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2018-09/txt/msg00468.txt.bz2 On 9/14/2018 11:26 AM, Simon Marchi wrote: > On 2018-09-12 09:01 PM, Weimin Pan wrote: >> Finding data member in virtual base class >> >> This patch fixes the original problem - printing member in a virtual base, >> using various expressions, do not yield the same value. Simple test case >> below demonstrates the problem: >> >> % cat t.cc >> struct base { int i; }; >> typedef base tbase; >> struct derived: virtual tbase { void func() { } }; >> int main() { derived().func(); } >> % g++ -g t.cc >> % gdb a.out >> (gdb) break derived::func >> (gdb) run >> (gdb) p i >> $1 = 0 >> (gdb) p base::i >> $2 = 0 >> (gdb) p derived::base::i >> $3 = 0 >> (gdb) p derived::i >> $4 = 4196392 >> >> To fix the problem, add function get_baseclass_offset() which searches >> recursively for the base class along the class hierarchy. If the base >> is virtual, it uses "vptr" in virtual class object, which indexes to >> its derived class's vtable, to get and returns the baseclass offset. >> If the base is non-virtual, it returns the accumulated offset of its >> parent classes. The offset is then added to the address of the class >> object to access its member in value_struct_elt_for_reference(). >> >> Tested on amd64-linux-gnu. No regressions. > I have some last nits about the test, but I'd like Tom to give the final approval, > since he clearly knows this stuff better than I do. If everything is fine with him, > then there is no need to post a new version, just fix it up and push. > >> --- >> gdb/ChangeLog | 7 +++ >> gdb/testsuite/ChangeLog | 6 +++ >> gdb/testsuite/gdb.cp/virtbase2.cc | 46 +++++++++++++++++++++ >> gdb/testsuite/gdb.cp/virtbase2.exp | 78 ++++++++++++++++++++++++++++++++++++ >> gdb/valops.c | 63 ++++++++++++++++++++++++++++- >> 5 files changed, 199 insertions(+), 1 deletions(-) >> create mode 100644 gdb/testsuite/gdb.cp/virtbase2.cc >> create mode 100644 gdb/testsuite/gdb.cp/virtbase2.exp >> >> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >> index c47c111..9cbd8ca 100644 >> --- a/gdb/ChangeLog >> +++ b/gdb/ChangeLog >> @@ -1,3 +1,10 @@ >> +2018-09-12 Weimin Pan >> + >> + PR gdb/16841 >> + * valops.c (get_virtual_base_offset): New function. >> + (value_struct_elt_for_reference): Use it to get virtual base offset >> + and add it in calculating class member address. >> + >> 2018-06-29 Pedro Alves >> >> * gdb/amd64-tdep.h (amd64_create_target_description): Add >> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog >> index 93c849c..46f16d7 100644 >> --- a/gdb/testsuite/ChangeLog >> +++ b/gdb/testsuite/ChangeLog >> @@ -1,3 +1,9 @@ >> +2018-09-12 Weimin Pan >> + >> + PR gdb/16841 >> + * gdb.cp/virtbase2.cc: New file. >> + * gdb.cp/virtbase2.exp: New file. >> + >> 2018-06-29 Pedro Alves >> >> * gdb.threads/names.exp: Adjust expected "info threads" output. >> diff --git a/gdb/testsuite/gdb.cp/virtbase2.cc b/gdb/testsuite/gdb.cp/virtbase2.cc >> new file mode 100644 >> index 0000000..60c95c4 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/virtbase2.cc >> @@ -0,0 +1,46 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> + Copyright 2018 Free Software Foundation, Inc. >> + >> + This program is free software; you can redistribute it and/or modify >> + it under the terms of the GNU General Public License as published by >> + the Free Software Foundation; either version 3 of the License, or >> + (at your option) any later version. >> + >> + This program is distributed in the hope that it will be useful, >> + but WITHOUT ANY WARRANTY; without even the implied warranty of >> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + GNU General Public License for more details. >> + >> + You should have received a copy of the GNU General Public License >> + along with this program. If not, see . */ >> + >> +struct superbase { >> + int x; >> + superbase () : x(22) {} >> +}; >> + >> +struct base : superbase { >> + int i; >> + double d; >> + base() : i(55), d(6.6) {} >> +}; >> + >> +typedef base tbase; >> +struct derived: virtual tbase >> +{ >> + void func_d() { } >> +}; >> + >> +struct foo: virtual derived >> +{ >> + void func_f() { } >> +}; >> + >> +int main() >> +{ >> + derived().func_d(); >> + foo().func_f(); >> +} >> + >> + > Remove the trailing empty lines here, git shows a warning when applying the patch: > > Applying: virtual inheritance via typedef cannot find base > Using index info to reconstruct a base tree... > M gdb/ChangeLog > M gdb/testsuite/ChangeLog > M gdb/valops.c > .git/rebase-apply/patch:90: new blank line at EOF. > + > warning: 1 line adds whitespace errors. > >> diff --git a/gdb/testsuite/gdb.cp/virtbase2.exp b/gdb/testsuite/gdb.cp/virtbase2.exp >> new file mode 100644 >> index 0000000..4c14419 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/virtbase2.exp >> @@ -0,0 +1,78 @@ >> +# Copyright 2018 Free Software Foundation, Inc. >> + >> +# This program is free software; you can redistribute it and/or modify >> +# it under the terms of the GNU General Public License as published by >> +# the Free Software Foundation; either version 3 of the License, or >> +# (at your option) any later version. >> +# >> +# This program is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> +# GNU General Public License for more details. >> +# >> +# You should have received a copy of the GNU General Public License >> +# along with this program. If not, see . >> + >> +# Make sure printing virtual base class data member works correctly (PR16841) >> + >> +if { [skip_cplus_tests] } { continue } >> + >> +standard_testfile .cc >> + >> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { >> + return -1 >> +} >> + >> +if {![runto_main]} then { >> + perror "couldn't run to main" >> + continue >> +} >> +> +proc make_scope_list { scopes } { > Please add a comment for this proc, since it's not very intuitive. Here's > a suggestion: > > # From a list of nested scopes, generate all possible ways of accessing something > # in those scopes. For example, with the argument {foo bar baz}, this proc will > # return: > # - {} (empty string) > # - baz:: > # - bar:: > # - bar::baz:: > # - foo:: > # - foo::baz:: > # - foo::bar:: > # - foo::bar::baz:: > > Thanks! > > Simon Hi Simon, OK, Thanks for your comments. I've incorporated them into the patch. Weimin