Re: [PATCH PR gdb/16841] virtual inheritance via typedef cannot find base

[Wei-min Pan <weimin.pan@oracle.com>]

On 6/16/2018 6:06 PM, Simon Marchi wrote:
> On 2018-06-15 07:45 PM, Weimin Pan wrote:
>> Fix failure to find member of a typedef base class
>>
>> The test case below demonstrates the problem, as described in this PR's Comment 5:
>>
>> typedef struct {
>>          int x;
>> } A;
>>
>> struct C : A {
>>          int y;
>> };
>>
>> int main()
>> {
>>          C c;
>>          return 55;
>> }
>>
>> $ gdb a.out
>> (gdb) ptype C::x
>> Internal error: non-aggregate type to value_struct_elt_for_reference
>>
>> In value_struct_elt_for_reference(), need to call check_typedef() on
>> the aggregate type to handle the case of *curtype being ptr->typedef.
> Thanks, that makes sense.  I noted some nits, you can push the patch after
> addressing them.
>
>> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
>> index b2938b1..78771d3 100644
>> --- a/gdb/testsuite/ChangeLog
>> +++ b/gdb/testsuite/ChangeLog
>> @@ -1,3 +1,9 @@
>> +2018-06-12  Weimin Pan  <weimin.pan@oracle.com>
>> +
>> +	PR gdb/16841
>> +	* gdb/testsuite/gdb.cp/typedef-base-ptype.cc: New file.
>> +	* gdb/testsuite/gdb.cp/typedef-base-ptype.exp: New file.
> Filenames should be relative to the ChangeLog file, so remove "gdb/testsuite/".
>
>> diff --git a/gdb/testsuite/gdb.cp/typedef-base-ptype.cc b/gdb/testsuite/gdb.cp/typedef-base-ptype.cc
>> new file mode 100644
>> index 0000000..bfbcc4a
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/typedef-base-ptype.cc
>> @@ -0,0 +1,30 @@
>> +/* 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 <http://www.gnu.org/licenses/>.  */
>> +
>> +typedef struct {
>> +        int x;
>> +} A;
>> +
>> +struct C : A {
>> +        int y;
>> +};
>> +
>> +int main()
>> +{
>> +        C c;
>> +        return 55;
>> +}
> We try to make the test code follow the GNU coding style (unless testing
> a particular formatting is the purpose of the test).
>
>> diff --git a/gdb/testsuite/gdb.cp/typedef-base-ptype.exp b/gdb/testsuite/gdb.cp/typedef-base-ptype.exp
> I would suggest naming this test "typedef-base.exp", because we may add other tests
> than ptype in it.  For example, David's original reproducer in PR 16841 still won't
> work after this patch (AFAIK), and would be a good candidate to end up as a test in
> this file.
>
>> new file mode 100644
>> index 0000000..e588fd3
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/typedef-base-ptype.exp
>> @@ -0,0 +1,37 @@
>> +# 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 <http://www.gnu.org/licenses/>.
> Can you add a one-liner comment that states the purpose of the test?  Something
> like
>
>    # Make sure that inheritance through a typedef is well handled.
>
> Thanks,
>
> Simon
>

Hi Simon,

Will incorporate your comments into the patch and push it.

Thanks,
Weimin